2

I have the following (simplified) code periodically run by a Thread in the class A (once per second):

Socket s = new Socket(IP,PORT);
ObjectOutputStream oos = new ObjectOutputStream(s.getOutputStream());
synchronized(this) {
   oos.writeObject(this);   //Exception HERE
   oos.flush();
}
...

The object (this) to send through the socket has an object of the class B as instance variable and B has a LinkedList<Long> as instance variable.

The application throws ConcurrentModificationException:

E/AndroidRuntime(681): FATAL EXCEPTION: Thread-10
E/AndroidRuntime(681): java.util.ConcurrentModificationException
E/AndroidRuntime(681): at java.util.LinkedList$LinkIterator.next(LinkedList.java:124)
E/AndroidRuntime(681): at java.util.LinkedList.writeObject(LinkedList.java:973)
E/AndroidRuntime(681): at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime(681): at java.lang.reflect.Method.invoke(Method.java:507)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1219)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1575)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1847)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1689)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1653)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeFieldValues(ObjectOutputStream.java:1143)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.defaultWriteObject(ObjectOutputStream.java:413)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1241)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1575)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1847)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1689)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1653)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeFieldValues(ObjectOutputStream.java:1143)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.defaultWriteObject(ObjectOutputStream.java:413)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1241)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1575)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1847)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1689)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1653)
E/AndroidRuntime(681): at java.util.LinkedList.writeObject(LinkedList.java:973)
E/AndroidRuntime(681): at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime(681): at java.lang.reflect.Method.invoke(Method.java:507)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1219)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1575)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1847)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1689)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1653)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeFieldValues(ObjectOutputStream.java:1143)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.defaultWriteObject(ObjectOutputStream.java:413)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1241)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1575)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1847)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1689)
E/AndroidRuntime(681): at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1653)
E/AndroidRuntime(681): at qoe.Application.connectUpdate(Application.java:194)
E/AndroidRuntime(681): at qoe.Application.access$0(Application.java:183)
E/AndroidRuntime(681): at qoe.Application$AutoUpdate.run(Application.java:217)

I run this app in Android with Eclipse and AVD, Windows 7 x64. Thanks in advance.

Edit: After many tests I think that the method that could cause problem is the following:

/* Instance variables */
private LinkedList<Long> mylist;
private long value;
/* The incriminated method */
public synchronized void myBadMethod() {
   this.mylist.add(this.value);
}
Baduel
  • 531
  • 3
  • 12
  • 30

4 Answers4

2

Clearly the list is being iterated during serialization, and simultaneously B is modifying the list. You need to synchronize those two bits of code so they cannot both execute at the same time.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • I forget to post in my question that BOTH the method are synchronized! – Baduel Nov 18 '11 at 00:37
  • 1
    @Baduel Obviously they can't be synchronized on the same object. I would synchronize them on the List object. – user207421 Nov 18 '11 at 00:44
  • Please note that this exception occurs also in the first 30 seconds (i.e. iterator is not used!). Also note that I modify a _copy_ of the original list with the iterator. – Baduel Nov 18 '11 at 00:47
  • 1
    @Baduel Then someone *else* is modifying the list during serialization. And you don't 'modify a copy of the original list'. You are only copying the reference, not the list. If you want to modify a copy, you haev to *make* a copy, e.g. with `new LinkedList(original_list)`. – user207421 Nov 18 '11 at 00:55
  • @Baduel Then you are mistaken, and it is `B` which is modifying the list. Once you fix it to modify a copy the problem should therefore disappear. Although what the purpose of modifying a copy may be is another mystery. Or else there is something strange about how the List is serialized. What kind of List implementation is it? – user207421 Nov 18 '11 at 02:19
  • I try to modify a copy in order to avoid this issue, but it seems that it is not the cause. I've posted class `B` before and after the issue. If you have 5 minutes please take a look. Thanks a lot. – Baduel Nov 18 '11 at 11:08
2

This bit of code:

LinkedList<Long> list_copy = this.original_list;
// ...
this.original_list = list_copy; // has no effect

is probably not doing what you think it is. The first line makes list_copy be a reference to the same list that this.original_list is a reference to. (In C++ terms, it's like writing list<long> * list_copy_ptr = this->original_list_ptr; it doesn't actually copy the list.) The last line really has no effect; it just makes this.original_list be a reference to the list it was already a reference to. (It might have an effect if you've got other unsynchronized code going on at the same time, but unless you're very careful with your volatiles, the effect isn't likely to be what you want.)

ruakh
  • 175,680
  • 26
  • 273
  • 307
  • To actually copy, you need list_copy = new ArrayList(original_list); – user949300 Nov 18 '11 at 01:00
  • Ok thanks, this is useful. But the Exception is thrown anyway. – Baduel Nov 18 '11 at 01:03
  • If somebody else is modifying original_list at the same time that you are copying it into list_copy that would happen. Try wrapping the copy in a synchronized(original_list) { } block. – user949300 Nov 18 '11 at 01:19
  • Or, if the if(something) path happens in the middle of Serialization, you end up changing original_list in the middle of Serialization, and I suspect (but do not know for sure) that that will cause problems. – user949300 Nov 18 '11 at 01:22
  • The real problem is that this Exception is thrown also if iterator is not used (I've performed different tests and I'm sure that in the first 30 seconds iterator is not used). Thus I think that the problem is in the serialization but I don't know with what is in conflict. – Baduel Nov 18 '11 at 01:36
  • @Baduel: So what you're saying is, you're sure that the problem isn't in the code that you've shown us. So what kind of answer are you expecting? No one here is clairvoyant, we can't help you find a bug in code we don't know about! – ruakh Nov 18 '11 at 01:40
  • @ruakh Yes, now I'm sure. I've commented the second piece of code and the problem is still here. You're right to say that in this way the question is not completed but I'm trying to draw conclusion with you, step by step. – Baduel Nov 18 '11 at 01:47
  • I've performed more tests and I have limited the problem to the `add` method of `LinkedList`. Please see the updated question for more details. – Baduel Nov 18 '11 at 12:44
0

You are modifying the list from two threads. The iterator class is not friendly to one thread modifying the List while it is iterating through it.

Use the keywork synchronized( object ) to mutually exclude the part of the code where each thread can conflict accessing the List data structure.

synchronized(this){ 
     Socket s = new Socket(IP,PORT); 
     ObjectOutputStream oos = new ObjectOutputStream(s.getOutputStream());    
     oos.writeObject(this);   //Exception HERE oos.flush(); ... 

}

and

 if(time < 30 seconds)    return; LinkedList<Long> list_copy =
 this.original_list;
     synchronized( this.Parent ){//where 'this' is class B and Parent is A
     Iterator<Long> it = list_copy.iterator();
     while(it.hasNext()) {
         Long current = it.next();
         if(something)
             it.remove();
     }
     this.original_list = list_copy; }
jeremyvillalobos
  • 1,795
  • 2
  • 19
  • 39
-1

Solved in 2 step.

1) I've used CopyOnWriteArrayList (a thread-safe variant of ArrayList). But in this way I had another issue: the operation remove by Iterator is not supported.

2) To resolve this issue I referred to this question:

LinkedList<Long> to_remove = new LinkedList<Long>();
    for(int i = 0; i < this.mylist.size(); i++) {

        Long current = this.mylist.get(i);
        if(conditionToRemove)
            to_remove.add(current);
    }
    this.mylist.removeAll(to_remove);

The drawback is that this is an expensive operation.

Community
  • 1
  • 1
Baduel
  • 531
  • 3
  • 12
  • 30