Concurrent list modification error.

[Richard BetsonParticipant]

Howdy,

I get a concurrent list modification error if I do something like this.

Monkey

Class Test End Class Global My_Test_List:=New List<Test> Local test_object:=New Test For Local myobject:Test=Eachin My_Test_List My_Test_List.Remove(myobject) Next

1 2 3 4 5 6 7 8 9 10 11 12

Class Test End Class   Global My_Test_List:=New List<Test>   Local test_object:=New Test   For Local myobject:Test=Eachin My_Test_List     My_Test_List.Remove(myobject) Next

Is this a bug or does Monkey 2 not support this as Monkey X did?

[peterigzParticipant]

You’ll need to do something along these lines:

Monkey

Local test_objects:= My_Test_List.All() While not test_objects.AtEnd local o:=nodes.Current test_objects.Erase() wend

1 2 3 4 5

Local test_objects:= My_Test_List.All() While not test_objects.AtEnd local o:=nodes.Current test_objects.Erase() wend

[Richard BetsonParticipant]

Well. I’ll give that a try…. but.

This issue is particularly confusing when converting Monkey X code. When Monkey 2 compiles code it will error on list.Remove() with no line reference in debug and just crashes the compiled application in release. So a user who does not know this is an issue is in some cases going to have a tough time figuring out what is going wrong. Believe me, I had a tough time narrowing the issue down with my large code base.

Anyway for continuities sake it might be a good idea to have Monkey 2 work as Monkey X did in this case.

[Mark SiblyKeymaster]

I wont be changing this – there’s just no easy way to guarantee you’re not completely messing up a list by removing stuff will you’re iterating through it, and I’m sure there’s a fair few ‘sneaky’ bugs out there due to this.

It showed up fine in the debugger for me, I just had to click on the first non-module function in the debug view.

Here’s a super-duper fast way to remove stuff while iterating through a stack:

Monkey

Local put:=0 For local item:=Eachin stack If item.ShouldBeRemoved continue stack[put]=item put+=1 Next stack.Resize( put )

1 2 3 4 5 6 7

Local put:=0 For local item:=Eachin stack    If item.ShouldBeRemoved continue    stack[put]=item    put+=1 Next stack.Resize( put )

[JesseParticipant]

@Richard
Your code on the first post works fine as is. I tried it and it does not give any errors compiling it with the latest MX2. The error must be somewhere else in your code.

[Mark SiblyKeymaster]

>Your code on the first post works fine as is

The code as posted doesn’t compile, and ‘fixing’ it still leaves you with an empty list! I had to add an item to break it.

It should also be mentioned that peterigz’s solution is faster – List.Remove() needs to find the item’s node using a linear search, while it.Remove() can just remove the node directly.

[JesseParticipant]

correction I did get the error by adding an item to the list:

Monkey

[crayon-5cba82df11561784243578 inline="true" ] #Import "<std>" Using std.. Class Test End Class Function Main() Global My_Test_List:=New List<Test> Local test_object:=New Test My_Test_List.AddLast(test_object) For Local myobject:Test=Eachin My_Test_List My_Test_List.Remove(myobject) Next End Function

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20

[crayon-5cba82df11561784243578 inline="true" ] #Import "<std>"   Using std.. Class Test End Class   Function Main() Global My_Test_List:=New List<Test> Local test_object:=New Test My_Test_List.AddLast(test_object) For Local myobject:Test=Eachin My_Test_List     My_Test_List.Remove(myobject) Next   End Function

[/crayon]

[JesseParticipant]

I guess we are crossing posts. Hahahah!

I usually only do it like this:

Monkey

[crayon-5cba82df13e75284464502 inline="true" ] If Not list.Empty Local link := list.FirstNode() While link.Value link.Remove() link = link.Succ Wend EndIf

1 2 3 4 5 6 7 8 9

[crayon-5cba82df13e75284464502 inline="true" ] If Not list.Empty   Local link := list.FirstNode()   While link.Value    link.Remove()    link = link.Succ   Wend EndIf

[/crayon]

This should be a bit faster. I suspect.

[Mark SiblyKeymaster]

Does that even work? Isn’t link.Succ null after the Remove()?

Weirdly, I have grown to really dislike  ‘stunt’ list processing!

And of course, List.Clear() is fastest…

[JesseParticipant]

> Does that even work? Isn’t link.Succ null after the Remove()?

You should know. you wrote it! :).
but yes it works.

>And of course, List.Clear() is fastest…
yes its faster. but for my usage which is not as simple as that, mine is a better option.

Monkey

[crayon-5cba82df189fc133671281 inline="true" ] If Not list.Empty Local link := list.FirstNode() While link.Value If link.Value.dead link.Remove() Endif link = link.Succ Wend EndIf

1 2 3 4 5 6 7 8 9 10 11 12

[crayon-5cba82df189fc133671281 inline="true" ] If Not list.Empty   Local link := list.FirstNode()   While link.Value    If link.Value.dead      link.Remove()    Endif    link = link.Succ   Wend EndIf

[/crayon]

I don’t see that as much of a stunt as its all part of List’s public functionality.

[Richard BetsonParticipant]

I’ll give the above samples a try. I’m cool with the fact that this this is how it has to be. I’ll adapt.

Thanks!

[Mark SiblyKeymaster]

> I’ll give the above samples a try.

The above code for iterating through a container is a bit wrong – here’a another example that also removes items as it goes:

Monkey

Local it:= My_List.All() While Not it.AtEnd Local item:=it.Current If item.dead it.Erase() Else it.Bump() Wend

1 2 3 4 5 6

Local it:= My_List.All() While Not it.AtEnd     Local item:=it.Current     If item.dead it.Erase() Else it.Bump() Wend

 

Note that you can replace List with Stack or Deque and it will look/work the same way.

I don’t see that as much of a stunt as its all part of List’s public functionality.

But it does depend on undocumented behaviour, eg: it assumes a node can still safely be used after it’s been removed from a list, and also that the ‘head’ node has a null value so you know when you’ve reached the end of list. This is what I mean by ‘stunt’ programming – relying on undocumented implementation details. There is a correct (ie: documented) way to do what you want (see above) so IMO it makes sense to use it.

In fact, your code *does* break here on the latest version, as I cleaned up node remove so that the node could still be inserted elsewhere safely, but I think I’ll give up on try to make a ‘clean’ list class and just concentrate on trying to duplicate bmx’s/monkey1’s quirks instead!

Think it’s also time to add RemoveIf() too!

[skn3Participant]

What if Stack (and others) had a new method for running an operation for each item in series.

For example:

Monkey

'this would be a member of Stack Method Do<T>:Void(callback:Void(item:T)) 'get list of all items Local items := ToArray() For Local index := 0 Until items.Length callback(items[index]) Next End 'example code Class Item Field name:String Method New(name:String) Self.name = name End End Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple")) items.Push(New Item("orange")) items.Push(New Item("tree")) items.Push(New Item("keyboard")) 'slow but works! items.Do(Lambda(item:Item) items.Remove(item) End) End

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32

'this would be a member of Stack Method Do<T>:Void(callback:Void(item:T)) 'get list of all items Local items := ToArray() For Local index := 0 Until items.Length callback(items[index]) Next End   'example code Class Item Field name:String Method New(name:String) Self.name = name End End   Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple")) items.Push(New Item("orange")) items.Push(New Item("tree")) items.Push(New Item("keyboard")) 'slow but works! items.Do(Lambda(item:Item) items.Remove(item) End) End

This could be used for a lot more then just removing!

Monkey

'example code Class Item Field name:String Field hitPoints:Int Method New(name:String, hitPoints:Int) Self.name = name Self.hitPoints = hitPoints End End Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple", 0)) items.Push(New Item("orange", 3)) items.Push(New Item("tree", 2)) items.Push(New Item("keyboard", 0)) Local text:String items.Do(Lambda(item:Item) If item.hitPoints <= 0 text+= ", "+item.name+" is dead" Endif End) Print text End

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27

'example code Class Item Field name:String Field hitPoints:Int Method New(name:String, hitPoints:Int) Self.name = name Self.hitPoints = hitPoints End End   Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple", 0)) items.Push(New Item("orange", 3)) items.Push(New Item("tree", 2)) items.Push(New Item("keyboard", 0)) Local text:String items.Do(Lambda(item:Item) If item.hitPoints <= 0 text+= ", "+item.name+" is dead" Endif End)     Print text End

 

Also clearly this is not the most efficient way to iterate and do stuff, but it works.

In these example the Do creates a list of items at the time of calling, so this is then safe to iterate over. It generates junk, but this could be optimised. Or a safe/unsafe flag could be specified, causing the loop to iterate directly, or create the array first with safe mode. e.g.

Monkey

Method Do<T>:Void(callback:Void(item:T),safe:bool= False) If safe 'iterate safe 'get list of all items Local items := ToArray() For Local index := 0 Until items.Length callback(items[index]) Next Else 'iterate unsafe For Local index := 0 Until _length callback(_data[index]) Next Endif End

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16

Method Do<T>:Void(callback:Void(item:T),safe:bool= False) If safe 'iterate safe 'get list of all items Local items := ToArray() For Local index := 0 Until items.Length callback(items[index]) Next Else 'iterate unsafe For Local  index := 0 Until _length callback(_data[index]) Next Endif End

 

Another example:

Monkey

Class Item Field name:String Field hitPoints:Int Method New(name:String, hitPoints:Int) Self.name = name Self.hitPoints = hitPoints End End Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple", 0)) items.Push(New Item("orange", 3)) items.Push(New Item("tree", 2)) items.Push(New Item("keyboard", 0)) Local currentOperation: = Operation1 items.Do(currentOperation) End Function Operation1:Void(item:Item) item.name = "NO" End Function Operation2:Void(item:Item) item.name = "YES" End Function Operation3:Void(item:Item) item.name = "MAYBE" End

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33

Class Item Field name:String Field hitPoints:Int Method New(name:String, hitPoints:Int) Self.name = name Self.hitPoints = hitPoints End End   Function Main:Void() Local items := New Stack<Item>() items.Push(New Item("apple", 0)) items.Push(New Item("orange", 3)) items.Push(New Item("tree", 2)) items.Push(New Item("keyboard", 0)) Local currentOperation: = Operation1 items.Do(currentOperation) End   Function Operation1:Void(item:Item) item.name = "NO" End   Function Operation2:Void(item:Item) item.name = "YES" End   Function Operation3:Void(item:Item) item.name = "MAYBE" End

[Richard BetsonParticipant]

but I think I’ll give up on try to make a ‘clean’ list class and just concentrate on trying to duplicate bmx’s/monkey1’s quirks instead!

Applause. I look at this issue as how best can MX2 provide continuity and how can MX2 provide intensives for BlitzMax users to climb on-board. One way is by including often used legacy commands and syntax where possible. I’m not selling the idea of backpedaling but making a case for simpler conversions of legacy code.

I’m in the middle of solidifying my converted code base, but when that is done I intend to explore Stac’s and determine where best to use them and what limitations they hold. I like List’s and when used properly it is a nice feature to have. After converting a ton of legacy code Monkey 2 has proven to be friendly to the process and support of List’s and some of the past quirks is a good Idea to further smoother legacy conversions.

[MikeHartParticipant]

How would someone implement an extention to a container while iterating through the same? Simple .AddLast or .Push/.Add?

[Author]

Posts