[Solved] problem with ClearStructure (v4.50b2 and v4.41)

[Demivec]

I've experienced some strange errors with ClearStructure() in both v4.41 and v4.50b2.

I've narrowed the code down to a small sample. It seems to occur with structure items that include strings.

Here's the sample code:

Code: Select all

#MaxItems = 7

Structure item
  enum.s ;Either "0" or "1"
EndStructure

Structure itemCollection
  Size.i ;zero based count of items present
  items.item[#MaxItems]
EndStructure


Procedure randomFill(*collection.itemCollection)
  ;fill the first 7 items of collection with either a "1" or "0"
  Protected i, enum.s
  *collection\Size = #MaxItems - 1 
  For i = 0 To *collection\Size
    If Random(1)
      enum = "1"
    Else
      enum = "0"
    EndIf 
    *collection\items[i]\enum = enum
  Next 
EndProcedure

Procedure.s showit(*collection.itemCollection)
  ;return a string containing the contents of collection in a comma delimeted list
  Protected i, output$
  
  For i = 0 To *collection\Size
    output$ + Chr(34) + *collection\items[i]\enum + Chr(34)
    If i <> *collection\Size: output$ + ", ": EndIf 
  Next
  ProcedureReturn output$
EndProcedure

;This procedure seems to be malfunctioning (notice all the debugs ;) )
;It mangles the contents of the collection on occasion.
Procedure removeOnes(*collection.itemCollection)
  ;remove all ones in collection
  Debug "": Debug "begin removal of 1's"
  Protected i, operation.s
  Debug "               " + showit(*collection)
  For i = *collection\Size To 0 Step -1
    If *collection\items[i]\enum = "1"
      Debug "  Clearing item #" + Str(i)
      ClearStructure(@*collection\items[i], item) ; has a problem here
      Debug "   clear struc " + showit(*collection)
      If i < *collection\Size
        MoveMemory(@*collection\items[i + 1], @*collection\items[i], SizeOf(item) * (*collection\Size - i)) ;no problems here
        operation = "   move data   "
      Else
        operation = "   adjust size "
      EndIf 
      *collection\Size - 1
      Debug operation + showit(*collection)
    EndIf 
  Next 
EndProcedure

Define a.itemCollection, i

RandomSeed(12) ;for results that are reproducable
For i = 0 To 100
  randomFill(a)
  removeOnes(a)
Next

The code repeatedly creates a random collection of items and then removes items from the collection until a fault occurs.

The removing occurs in the procedure removeOnes(). This procedure clears the structure of matching items and then either overwrites them with the remaining items using movememory() or adjusts the count of items if it is the last item being removed.

Occasionally ClearStructure() produces odd and soon to be fatal results, especially when items are being removed from the end of the collection. In many cases if there are multiple items the same it will remove all of them in one swoop (without being told to do so).


Can anyone offer an explanation or point out where the program logic is flawed? I don't need a way to accomplish the task another way (thanks anyway ), I am just trying to narrow down a possible bug. Any insight would be appreciated.

@Edit: it appears the problem is with a duplication of a structure(s) as a side-effect of the use of MoveMemory(). Added [Solved] to title.

[luis]

Uhmmm... some unrelated ideas:

First: I'm not sure ClearStructure() is intended to be used that way. I mean... I THOUGHT if you use CS() on a memory area (and I would expect that to be dynamically allocated) you shouldn't reference that again after that.

But that's how I percieved CS() and I could be wrong.

Second:

I'm not sure your use of movememory() is ok. Aren't you creating aliases of the same structure over and over that way? I suppose a PB string has a pointer to memory associated, and when you copy an "item"s structure from one place to another inside the host structure, aren't you cloning one pointer in another position... ? Maybe I should try to understand better your code but I got this impression.

Change showit() this way

Code: Select all

Procedure.s showit(*collection.itemCollection)
  ;return a string containing the contents of collection in a comma delimeted list
  Protected i, output$
  
  For i = 0 To #MaxItems - 1
    Debug Str(i) + " -> " + Str(@*collection\items[i]\enum)
  Next 
  
  For i = 0 To *collection\Size
    
    output$ + Chr(34) + *collection\items[i]\enum + Chr(34)
    If i <> *collection\Size: output$ + ", ": EndIf
  Next
  
  ProcedureReturn output$
EndProcedure

You should see the base address of each "enum" string be cloned sooner or later, and after that when you write to the item you are in reality writing to many fields at once.

But I admit I'm not sure of what ClearStructure() is expected to do when used this way also (I mean not with a pointer to allocated memory that it will be freed right after the CS )

Not sure if I'm helping or not

[breeze4me]

I'm not sure this is the correct way, but it seems to be fine.

Code: Select all

Procedure removeOnes(*collection.itemCollection)
  ;remove all ones in collection
  Debug "": Debug "begin removal of 1's"
  Protected i, operation.s
  Protected *tmp = AllocateMemory(SizeOf(item))
  
  Debug "               " + showit(*collection)
  For i = *collection\Size To 0 Step -1
    If *collection\items[i]\enum = "1"
      Debug "  Clearing item #" + Str(i)
      ClearStructure(@*collection\items[i], item) ; has a problem here
      Debug "   clear struc " + showit(*collection)
      If i < *collection\Size
        CopyMemory(@*collection\items[i], *tmp, SizeOf(item)) ; copy a freed structure to *tmp
        MoveMemory(@*collection\items[i + 1], @*collection\items[i], SizeOf(item) * (*collection\Size - i))
        CopyMemory(*tmp, @*collection\items[i] + SizeOf(item) * (*collection\Size - i), SizeOf(item)) ;to delete previous memory contents, restore *tmp to new position
        operation = "   move data   "
      Else
        operation = "   adjust size "
      EndIf 
      *collection\Size - 1
      Debug operation + showit(*collection)
    EndIf 
  Next 
  
  FreeMemory(*tmp)
EndProcedure

[Demivec]

Uhmmm... some unrelated ideas:

First: I'm not sure ClearStructure() is intended to be used that way. I mean... I THOUGHT if you use CS() on a memory area (and I would expect that to be dynamically allocated) you shouldn't reference that again after that.

But that's how I percieved CS() and I could be wrong.

@luis: ClearStructure() zeroes numeric data, unallocates string data, and I believe calls FreeMap(), FreeList(), FreeArray() on those respective sub-structures. It can be used with an allocated memory area or an already existing one. PureBasic should be doing something like ClearStructure() under the hood when a variable goes out of scope.

The error seems to lie in what you and breeze4me found with MoveMemory(). I had incorrectly made a duplicate of the last structure being 'moved'. As breeze4me demonstrated, it needed to be overwritten with a copy of a cleared structure. Since any cleared structure would do I adapted his solution to copy a empty structured variable:

Code: Select all

Procedure removeOnes(*collection.itemCollection)
  ;remove all ones in collection
  Debug "": Debug "begin removal of 1's"
  Protected i, operation.s,tmp.item
  
  Debug "               " + showit(*collection)
  For i = *collection\Size To 0 Step -1
    If *collection\items[i]\enum = "1"
      Debug "  Clearing item #" + Str(i)
      ClearStructure(@*collection\items[i], item) ; has a problem here
      Debug "   clear struc " + showit(*collection)
      If i < *collection\Size
        MoveMemory(@*collection\items[i + 1], @*collection\items[i], SizeOf(item) * (*collection\Size - i))
        CopyMemory(@tmp, @*collection\items[i] + SizeOf(item) * (*collection\Size - i), SizeOf(item)) ;to delete previous memory contents, restore tmp to new position
        operation = "   move data   "
      Else
        operation = "   adjust size "
      EndIf
      *collection\Size - 1
      Debug operation + showit(*collection)
    EndIf
  Next
EndProcedure

Thanks for the additional brain power. I've added [Solved] to the title. .

[ts-soft]

@luis: ClearStructure() zeroes numeric data, unallocates string data, and I believe calls FreeMap(), FreeList(), FreeArray() on those respective sub-structures.

Clear <> Free!

[Demivec]

@luis: ClearStructure() zeroes numeric data, unallocates string data, and I believe calls FreeMap(), FreeList(), FreeArray() on those respective sub-structures.

Clear <> Free!

Good point. How do you explain this code then?

Code: Select all

Structure stuff
  List b.s()
EndStructure

Define a.stuff

CallDebugger
AddElement(a\b())
a\b() = "happy"

ForEach a\b()
  Debug a\b()
Next

ClearStructure(@a,stuff) ;this frees the list...

;NewList a\b() ;uncomment this is recreate the freed list so elements can be added
AddElement(a\b()) : a\b() = "day"
ForEach a\b()
  Debug a\b()
Next

[luis]

It can be used with an allocated memory area or an already existing one. PureBasic should be doing something like ClearStructure() under the hood when a variable goes out of scope.

Yup, you are right. For some reason I had the suspect it was intended only for dinamically allocated structures but it's not so. Thanks

[ts-soft]

Good point. How do you explain this code then?

I think this is a bug! Only ClearList() and not FreeList() should called.

[Demivec]

Good point. How do you explain this code then?

I think this is a bug! Only ClearList() and not FreeList() should called.

I also think it is something that needs to be cleared up. I am in agreement with you. I simply put the sample code to demonstrate what I saw happening.

I am waiting for additional information to be disseminated. I know there are several related issues that need to be addressed also, they've been mentioned in their own topics already. You may want to mentioned this one.

[freak]

ClearStructure() cleans up the content of a structure so its memory can be released without creating leaks. It is intended to be used on allocated memory, not on a normal structured variable (cleanup is automatic here).

If it just did a ClearList() then you would leak the list header every time. If you want to re-use the memory again, you can call InitializeStructure() again.

[ts-soft]

Thank you for the explanation

[Demivec]

Thanks for this additional explanation. It would seem I had mis-used ClearStructure() also. I'll make a note of it.

[luis]

ClearStructure() cleans up the content of a structure so its memory can be released without creating leaks. It is intended to be used on allocated memory, not on a normal structured variable (cleanup is automatic here).

That was my original thought, but after the question posted by Demivec I tried on a normal variable and actually seemed to work without visible side effects. Are there any ?

Not that I'm planning to use it that way. Just to understand.



Based on what you said, probably would be better to change:

"ClearStructure can be used to clear a structured memory area. It's for advanced use only, when pointers are involved"

to

"ClearStructure can be used to clear a structured memory area. It's for advanced use only, when dinamically allocated structures are involved"

in the manual...

[Demivec]

@luis: I agree the manual gave me a wrong impression for ClearStructure(). It gives an example there which does not use pointers. Perhaps it's okay to use ClearStructure() if there are no lists, maps, or arrays (that's what I did).

Here is a copy of that example:

Code: Select all

Structure People
    Name$
    LastName$
    Age.l
EndStructure

Student.People\Name$ = "Paul"
Student\LastName$ = "Morito"
Student\Age = 10
  
ClearStructure(@Student, People)
  
; Will print empty strings as the whole structure has been cleared. All other fields have been resetted to zero.
;
Debug Student\Name$
Debug Student\LastName$
Debug Student\Age

It definitely needs to be updated to reflect what freak said.

[Seymour Clufley]

It definitely needs to be updated to reflect what freak said.

+1

The help is misleading in this instance.