Java-Gaming.org Hi !
Featured games (91)
games approved by the League of Dukes
Games in Showcase (798)
Games in Android Showcase (234)
games submitted by our members
Games in WIP (865)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1] 2
  ignore  |  Print  
  Concurrent Modification when changing maps?  (Read 13941 times)
0 Members and 1 Guest are viewing this topic.
Offline MrPork
« Posted 2016-12-05 06:02:24 »

I created a simple system to change levels. Upon reaching level 2 I get a concurrent modification error any time I try to erase all the objects in my list. Using an iterator doesn't work, using ArrayList.clear() doesn't work either. Iv'e exhausted all my options as a nub, what gives?

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  
public void update(float delta){
     
      if(MapManager.isLoading){
         
         for(int i = 0; i < itemList.size(); i++){
            itemList.remove(i);
         }
         
         MapManager.isLoading = false;
         
      }
     
   if(!MapManager.isLoading){
      for(Iterator<Item> itemIter = itemList.iterator(); itemIter.hasNext();){
         Item item = itemIter.next();
         item.update(delta);
         
         if(!item.isAlive()){
            itemIter.remove();
         }
         
         if(EntityManager.playerReference.getRectangle().overlaps(item.getRectangle())){
            item.doSomething();
         }
         
          }
      }
   }
   
}

"f**k it, maybe it'll work." -Me
Offline VaTTeRGeR
« Reply #1 - Posted 2016-12-05 09:41:21 »

Does the first loop work correctly?

Say you have 2 items in that list:
1  
2  
iteration 1: i = 0; itemList.size()=2 -> remove 0
iteration 2: i = 1; itemList.size()=1 -> does not get executed, one item left!


So shouldn't it be:

1  
2  
3  
for(int i = itemList.size() - 1; i >= 0; i--) {
    itemList.remove(i);
}


or

1  
itemList.clear();

Offline h.pernpeintner

JGO Ninja


Medals: 107



« Reply #2 - Posted 2016-12-05 09:45:34 »

You (ususally) cannot remove objects from a collection you are currently iterating over. If you want to clear a list completely, use .clear(). Otherwise, create a list where you collect references to the entitites you want to delte in a first iteration and delete them in a second iteration with .remove(entity).

To be more precise:


for(int i = 0; i < itemList.size(); i++){
  itemList.remove(i);
}

can be replaced with itemList.clear();


If you still get ConcurrentModException, you are iterating over itemList somewhere else Smiley
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline VaTTeRGeR
« Reply #3 - Posted 2016-12-05 10:04:33 »

Quote
If you still get ConcurrentModException, you are iterating over itemList somewhere else  Smiley

That is the most plausible cause if the issue still persists after replacing the first loop with "itemList.clear();"

His second loop looks correct to me, only thing that is redundant is the "if(!MapManager.isLoading)" statement because the loop gets executed anyway, "MapManager.isLoading" always gets set to false in the first block when it is true in the beginning.
Offline VoidBuffer

Junior Devvie


Medals: 4
Exp: 2 years



« Reply #4 - Posted 2016-12-05 17:48:26 »

In addition to the discussions above, you could also replace the first for loop with an Iterator that removes from your list, just like your second for loop. See the following: http://stackoverflow.com/questions/18448671/how-to-avoid-concurrentmodificationexception-while-removing-elements-from-arr
Offline MrPork
« Reply #5 - Posted 2016-12-06 00:24:10 »

Well I have tried using itemList.clear() but it gives me the same error. In my map changing code I have the below, and it succeeds in adding them to the list but as soon as they start updating it crashes and gives me a concurrent modification error.

Update : I checked to see if the itemList was of the correct size and it is. It's only after adding the objects that it completely boots itself.


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  
34  
35  
36  
37  
38  
39  
40  
41  
42  
43  
44  
45  
46  
47  
48  
49  
50  
51  
52  
   public static void loadMapItems(TiledMap tiledMap, TiledMapRenderer renderer) {
      isLoading = true;
      currentRenderer = renderer;

     
      ItemManager.itemList.clear();
     
      if (tiledMap != null) {
         MapLayer collisionObjectLayer = tiledMap.getLayers().get("Objetos");
         MapObjects objects = collisionObjectLayer.getObjects();

         for (MapObject Object : objects.getByType(MapObject.class)) {
            float posX = Object.getProperties().get("x", float.class);
            float posY = Object.getProperties().get("y", float.class);

            if (Object.getProperties().get("item") != null) {

               if (Object.getProperties().get("item").equals("moneda")) {
                  ItemManager.itemList.add(new ItemMoneda(posX, posY));
                  System.out.println("Item :Moneda: created");
               }

               if (Object.getProperties().get("item").equals("mina")) {
                  ItemManager.itemList.add(new ItemMina(posX, posY));
                  System.out.println("Item :Mina: Created");
               }

               if (Object.getProperties().get("item").equals("Santi")) {
                  ItemManager.itemList.add(new ItemSanti(posX, posY));
                  System.out.println("Item :Santi: Created");
               }

            }

            if (Object.getProperties().get("enemy") != null) {

               if (Object.getProperties().get("enemy").equals("malo")) {
                  ItemManager.itemList.add(new ItemMalo(posX, posY));
                  System.out.println("Item :Malo: Created");
               }

            }

         }
      }
     
   
     
      System.out.println("toot");
      isLoading = false;

   }

"f**k it, maybe it'll work." -Me
Offline VaTTeRGeR
« Reply #6 - Posted 2016-12-06 07:47:11 »

That's fine and all, but what exactly are you doing concurrently?
All the code you provided does in no way enable anyone to help you with your core issue...

When are you calling which method? What is the call hierachy? Are you using more than one thread?
Offline Abuse

JGO Ninja


Medals: 71


falling into the abyss of reality


« Reply #7 - Posted 2016-12-06 12:09:05 »

Looks to me like you're attempting to use the isLoading flag as a mutex.

That's unlikely to be thread safe.
You need to use proper synchronization.
Offline MrPork
« Reply #8 - Posted 2016-12-06 18:01:08 »

The only times I iterate over the itemList is when I'm rendering() and updating() the objects. They are not called elsewhere but in the class. I am in the process of using synchronization to see if @Abuse 's help works.

"f**k it, maybe it'll work." -Me
Offline MrPork
« Reply #9 - Posted 2016-12-06 18:04:07 »

I am also not using multiple threads.

"f**k it, maybe it'll work." -Me
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline MrPork
« Reply #10 - Posted 2016-12-06 18:48:44 »

Looks to me like you're attempting to use the isLoading flag as a mutex.

That's unlikely to be thread safe.
You need to use proper synchronization.

Synchronization did not work.

"f**k it, maybe it'll work." -Me
Offline Kefwar
« Reply #11 - Posted 2016-12-06 18:53:00 »

...
Synchronization did not work.
Well, since you said you were not using multiple threads you don't have to bother with synchronization.
Providing additional information may be useful if you want to receive help, such as a stack trace including line numbers in your code snippets.
At least for me it isn't clear where the error exactly occurs.

Offline Abuse

JGO Ninja


Medals: 71


falling into the abyss of reality


« Reply #12 - Posted 2016-12-06 19:49:44 »

I am also not using multiple threads.

So where else is isLoading used?
If there aren't other threads referencing it, then it's completely superfluous.
If there are other threads referencing it, then it's almost certainly unsafe.
Either way, it's a pointer to a problem in design or understanding.

Rather than giving us a useless fragment of code, and having us guess at the cause, stick your code on github in its entirety so we can tell you precisely what the problem is.

Alternatively, learn to use a debugger.
Offline philfrei
« Reply #13 - Posted 2016-12-06 22:35:58 »

I wonder if this is a job for CopyOnWriteArrayList?

If iterations outnumber mutations (which is usually the case if you are using the list in a render loop), then this could be just the ticket.

It pretty much works the same as an ArrayList, but with elements that make it thread safe.

I think we are getting to the point where almost any use of Synchronization is suspect, given all the new thread-safe and functional-programming based tools that have been added to Java.


music and music apps: http://adonax.com
Offline philfrei
« Reply #14 - Posted 2016-12-06 22:38:48 »

I am also not using multiple threads.

The JVM begs to differ:
Quote
I get a concurrent modification error ...

music and music apps: http://adonax.com
Offline Abuse

JGO Ninja


Medals: 71


falling into the abyss of reality


« Reply #15 - Posted 2016-12-07 00:34:58 »

I am also not using multiple threads.

The JVM begs to differ:
Quote
I get a concurrent modification error ...

Not strictly true.

Quote
Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception.

The provided code fragment could be doing this, but there's no way for us to know if collisionObjectLayer.getObjects() is the same Collection as ItemManager.itemList
Offline cylab

JGO Kernel


Medals: 195



« Reply #16 - Posted 2016-12-07 08:09:31 »

You really need to add stacktraces and correct line numbers to this sort of questions. This way we are just guessing.

But since I am usually good at guessing, I'll give it a try: could it be, that you are adding items to the collisionObjectsLayer, when you are adding them to the ItemManager.itemList?

Mathias - I Know What [you] Did Last Summer!
Offline Stranger
« Reply #17 - Posted 2016-12-08 20:40:07 »

Maybe you should consider using two lists for the game entities?

1  
2  
List<Entity> oldList; 
List<Entity> newList;


I hope you can get rid from your persuasive problems ...

http://stackoverflow.com/questions/1667481/compare-two-lists-for-updates-deletions-and-additions
http://gamedev.stackexchange.com/questions/2647/how-do-i-best-remove-an-entity-from-my-game-loop-when-it-is-dead

Anton
Offline Riven
Administrator

« JGO Overlord »


Medals: 1369
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #18 - Posted 2016-12-08 21:19:59 »

Using an iterator doesn't work, using ArrayList.clear() doesn't work either.
You said in your opening post that ArrayList.clear() fails with ConcurrentModificationException.

This PROVES multiple threads are modifying the ArrayList.

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings!
Offline basil_

« JGO Bitwise Duke »


Medals: 418
Exp: 13 years



« Reply #19 - Posted 2016-12-08 23:01:55 »

isn't that just a mod-count check, which could be created by something in the loop ?
Offline princec

« JGO Spiffy Duke »


Medals: 1107
Projects: 3
Exp: 20 years


Eh? Who? What? ... Me?


« Reply #20 - Posted 2016-12-08 23:55:19 »

This PROVES multiple threads are modifying the ArrayList.
No, it just proves he's trying to iterate through a list at the same time as adding or removing things to it.
The classic mistake:
1  
2  
3  
4  
5  
6  
7  
      ArrayList<String> strings = new ArrayList<>(Arrays.asList("woops", "this", "is", "wrong"));

      for (String s : strings) {
         if ("wrong".equals(s)) {
            strings.add("kaboom");
         }
      }

Boom! CME.

Cas Smiley

Offline philfrei
« Reply #21 - Posted 2016-12-09 04:53:40 »

Use a stream?

1  
strings.forEach( Your-Consumer-here );


At least, maybe, use them for the *theoretically* simple stuff like iterating for a render or update. (Assumes they don't overlap--which they shouldn't if you designed things right.)

music and music apps: http://adonax.com
Offline princec

« JGO Spiffy Duke »


Medals: 1107
Projects: 3
Exp: 20 years


Eh? Who? What? ... Me?


« Reply #22 - Posted 2016-12-09 09:39:11 »

Also wouldn't it be nice to be able to just write
1  
ArrayList<String> strings = ["hurrah", "for", "compilers"];

and have it figure out you want to instantiate an ArrayList from that?
While we're at it
1  
HashMap<String, String> things = ["Don't":"hold", "your":"breath"];

Ho hum.

Cas Smiley

Offline pateman

Senior Newbie


Exp: 6 years



« Reply #23 - Posted 2016-12-09 10:19:43 »

Also wouldn't it be nice to be able to just write
1  
ArrayList<String> strings = ["hurrah", "for", "compilers"];

and have it figure out you want to instantiate an ArrayList from that?
While we're at it
1  
HashMap<String, String> things = ["Don't":"hold", "your":"breath"];

Ho hum.

Cas Smiley

Don't want to start a war or something, but Groovy has that Wink
Offline Riven
Administrator

« JGO Overlord »


Medals: 1369
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #24 - Posted 2016-12-09 22:47:56 »

This PROVES multiple threads are modifying the ArrayList.
No, it just proves he's trying to iterate through a list at the same time as adding or removing things to it.
The classic mistake: [snip]

No, when ArrayList.clear() (not remove, not add: clear!!) fails with ConcurrentModificationException, the current thread is not also adding/removing *during* the clear, hence another thread is, hence the proof.

Using an iterator doesn't work, using ArrayList.clear() doesn't work either. Iv'e exhausted all my options as a nub, what gives?


Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings!
Offline cylab

JGO Kernel


Medals: 195



« Reply #25 - Posted 2016-12-09 23:44:08 »

No, when ArrayList.clear() (not remove, not add: clear!!) fails with ConcurrentModificationException, the current thread is not also adding/removing *during* the clear, hence another thread is, hence the proof.

I doubt, the CME was raised in clear(). We didn't see a stacktrace from OP, so I still bet it's while adding to the itemList...

Mathias - I Know What [you] Did Last Summer!
Offline princec

« JGO Spiffy Duke »


Medals: 1107
Projects: 3
Exp: 20 years


Eh? Who? What? ... Me?


« Reply #26 - Posted 2016-12-10 00:12:45 »

But ArrayList.clear() cannot throw CME? Only the Iterator methods actually do the throwing - the ArrayList stuff just sets it up to throw next time one of them is called.

Cas Smiley

Offline Stranger
« Reply #27 - Posted 2016-12-10 11:45:54 »

ArrayList#clear()
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
/**
 * Removes all of the elements from this list.  The list will
 * be empty after this call returns.
 */

   public void clear() {
        modCount++;

        // clear to let GC do its work
        for (int i = 0; i < size; i++)
            elementData[i] = null;

        size = 0;
}


Riven is the boss.
The boss is always right.
If the boss is really right he's right even more =>
Riven is right. (Look upward at the code)
This completes the proof.

Btw, I offered something wrong about added/removed lists?

Anton
Offline princec

« JGO Spiffy Duke »


Medals: 1107
Projects: 3
Exp: 20 years


Eh? Who? What? ... Me?


« Reply #28 - Posted 2016-12-10 16:33:02 »

Have I suddenly entered a parallel universe?

Cas Smiley

Offline Riven
Administrator

« JGO Overlord »


Medals: 1369
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #29 - Posted 2016-12-10 20:31:33 »

Nope! He just poked fun of me, given that I failed miserably Smiley

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings!
Pages: [1] 2
  ignore  |  Print  
 
 

 
Riven (81 views)
2019-09-04 15:33:17

hadezbladez (4337 views)
2018-11-16 13:46:03

hadezbladez (1547 views)
2018-11-16 13:41:33

hadezbladez (4462 views)
2018-11-16 13:35:35

hadezbladez (871 views)
2018-11-16 13:32:03

EgonOlsen (4152 views)
2018-06-10 19:43:48

EgonOlsen (4855 views)
2018-06-10 19:43:44

EgonOlsen (2803 views)
2018-06-10 19:43:20

DesertCoockie (3707 views)
2018-05-13 18:23:11

nelsongames (3998 views)
2018-04-24 18:15:36
Java Gaming Resources
by philfrei
2019-05-14 16:15:13

Deployment and Packaging
by philfrei
2019-05-08 15:15:36

Deployment and Packaging
by philfrei
2019-05-08 15:13:34

Deployment and Packaging
by philfrei
2019-02-17 20:25:53

Deployment and Packaging
by mudlee
2018-08-22 18:09:50

Java Gaming Resources
by gouessej
2018-08-22 08:19:41

Deployment and Packaging
by gouessej
2018-08-22 08:04:08

Deployment and Packaging
by gouessej
2018-08-22 08:03:45
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!