› › ›

ArrayList - ConcurrentModificationException

vjjvvjjv
edited March 2018 in Questions about Code

hey, im trying to add Objects into an arrayList that is a part of a class, from a second Papplet,(when the user click the mouse, add a new Obj). But it doesnt work properly, an concurrent modification exception is crashing the sketch:

here the code:

the main scene 3D

System s;
PWindow w;
void settings() {
  s = new System();
  size(500, 500, P3D);

}

void setup() {
  background(0);
  s = new System();
  w = new PWindow();
}

void draw() {
  background(0);
  pushMatrix();
  translate(width/2, height/2);
  s.run();
  popMatrix();
}

class System {
  ArrayList<Obj>o;
  System() {
    o = new ArrayList<Obj>();
  }

  void addObj(float x, float z) {
    o.add(new Obj(new PVector(x, 0, z)));
  }


  void run() {
    for (Obj obj : o) {
      obj.display();
    }
  }

}

class Obj {

  PVector pos;

  Obj(PVector o) {
    pos = o.get();
  }

  void display() {
    pushMatrix();
    translate(pos.x, pos.y, pos.z);
    sphere(100);
    popMatrix();
  }
}

the second Papplet , when the user click the screen, add a new Obj

    class PWindow extends PApplet {

      PWindow() {
        super();
        PApplet.runSketch(new String[] {this.getClass().getSimpleName()}, this);
      }


      void settings() {
        size(500, 500, P2D);
      }

      void setup() {
      }

      void draw() {
        for(Obj o : s.o){
        ellipse(o.pos.x, o.pos.z, 20, 20);
        }
      }

      void mousePressed(){
      s.addObj(mouseX,mouseY);
      }


    }
Tagged:

Best Answer

Answers

  • ChrisirChrisir

    why do you want a second papplet?

  • GoToLoopGoToLoop
    edited March 2018

    I'm even surprised your code didn't outright crash earlier! @-)

    B/c as far as I knew up till now, we can't have more than 1 PApplet instance using an OpenGL-based renderer, like P2D & P3D, in the same sketch. Same for the FX2D renderer too! ~:>

    But running your code under PDE version 3.3.5, it didn't crash at the beginning as it would under older versions! :-/

    Even though you've got 2 OpenGL-based renderers right there! P3D & P2D. 8-}
    Maybe just recently they've inadvertently fixed that by sheer chance? :-j

    Moreover, we generally give plural or collective names for variables holding containers: like arrays, lists, etc. :-B

    However, you've got a List field simply named as o there inside your System class! 8-|

    Also Obj isn't very descriptive for a class either. You should consider better names for them! [-O<

    P.S.: A new PDE version 3.3.7 (2018/Mar/13) is available now! <:-P
    https://Processing.org/download/

    As for moi, I'm gonna wait a lil' longer before getting it in order to see whether any1 shows up here about bugs on this latest version. >:)

    I've completely skipped version 3.3.6 b/c lotsa folks had problems w/ that 1. 8-X

  • GoToLoopGoToLoop
    edited March 2018

    Regardless all I've said on my 1st reply above, your actual immediate problem is about ConcurrentModificationException, right? :>
    https://Docs.Oracle.com/javase/9/docs/api/java/util/ConcurrentModificationException.html

    You've got 2 PApplet instances running concurrently almost at the same time.

    What happens is that your 2nd PApplet, that is your subclass PWindow, may invoke System::addObj() inside its own mousePressed() callback at the same time the main PApplet is busy iterating over the container o via System::run() inside its draw() callback. :-B

    Your method System::addObj() invokes method Collection::add():
    https://Docs.Oracle.com/javase/9/docs/api/java/util/Collection.html#add-E-

    Which of course, increments its internal Collection::size():
    https://Docs.Oracle.com/javase/9/docs/api/java/util/Collection.html#size--

    If that happens to be at the same time the container is busy being iterated over by a loop, a ConcurrentModificationException is highly probable to be thrown! :-&

    B/c its Collection::size() has changed. And the loop had started w/ a diff. Collection::size() value. #-o

  • GoToLoopGoToLoop
    edited March 2018

    • As a workaround, I've rewritten your System class.

    • Now, its 2 methods addObj() & run() got their own synchronized () {} block.

    • Under System::addObj(), synchronized () {} protects the Collection::add() call.

    • And under System::run(), it protects its full iteration loop.

    • More precisely, if 1 Thread is already running either System::addObj() or System::run(), another Thread gotta await for it to finish before running either of those as well. :ar!

    • For both synchronized () {} blocks, I've chosen the List o's reference as its monitor/gatekeeper.

    • Anyways, please rename your field o and your class Obj w/ more proper names later. ;)

    import java.util.List;

class System {
  final List<Obj> o = new ArrayList<Obj>();
  final PVector v = new PVector();

  void addObj(final float x, final float z) {
    synchronized (o) {
      final Obj obj = new Obj(v.set(x, 0, z));
      o.add(obj);
    }
  }

  void run() {
    synchronized (o) {
      for (final Obj obj : o)  obj.display();
    }
  }
}

    BtW, here's some synchronized quick tutorial, just so you can have some basic idea about it: ~O)
    https://Docs.Oracle.com/javase/tutorial/essential/concurrency/locksync.html

  • GoToLoopGoToLoop
    edited March 2018 Answer ✓

    And here's a more simplified version, in which the whole method is declared as synchronized rather than having synchronized () {} blocks inside it. \m/

    import java.util.List;

class System {
  final List<Obj> o = new ArrayList<Obj>();
  final PVector v = new PVector();

  synchronized void addObj(final float x, final float z) {
    final Obj obj = new Obj(v.set(x, 0, z));
    o.add(obj);
  }

  synchronized void run() {
    for (final Obj obj : o)  obj.display();
  }
}

    P.S.: System is another bad name! Even more considering Java already got a class called System: :-@
    https://Docs.Oracle.com/javase/9/docs/api/java/lang/System.html

  • vjjvvjjv

    thank you very much!

    i also fix the problem iterating backwards:

      void run() {
    for (int i = o.size()-1; i >= 0; i--) {
      Obj obj = o.get(i);
      obj.display();
    }
  }

    :) cheers!

  • GoToLoopGoToLoop
    edited March 2018

    Indeed, a backwards loop can be a workaround for Collection::add(). :ar!

    However, if other threads invoke other Collection::size() mutable methods, such as List::remove() or, even worse, Collection::clear(): L-)

    1. https://Docs.Oracle.com/javase/9/docs/api/java/util/List.html#remove-int-
    2. https://Docs.Oracle.com/javase/9/docs/api/java/util/Collection.html#clear--

    If the removed element was the last 1 (tail element), and the backwards loop had already started, and is yet about to invoke List::get() method for the very 1st time: :-SS
    https://Docs.Oracle.com/javase/9/docs/api/java/util/List.html#get-int-

    B/c the tail element doesn't exist anymore, an IndexOutOfBoundsException is gonna be thrown: :-&
    https://Docs.Oracle.com/javase/9/docs/api/java/lang/IndexOutOfBoundsException.html

    Obviously, that's a very slim chance. Plus for now, your sketch just use Collection::add(). :P

    But I thought you should know about such peculiar risk:
    That even backwards loop can fail some times under a multi-threaded environment. >-)

Sign In or Register to comment.