Opened 6 years ago

Closed 6 years ago

#1703 closed defect (fixed)

I2P-Bote concurrent.ExecutionException

Reported by: killyourtv Owned by: str4d
Priority: minor Milestone: 0.9.23
Component: apps/plugins Version: 0.9.22
Keywords: I2P-Bote Cc: str4d
Parent Tickets: Sensitive: no


I2P version: 0.9.22-21-deb1
Java version: Oracle Corporation 1.8.0_66-internal (OpenJDK Runtime Environment 1.8.0_66-internal-b17)
Wrapper version: 3.5.26
Server version: 8.1.17.v20150415
Servlet version: Jasper JSP 2.1 Engine
Platform: Linux amd64 4.2.0-1-amd64
Processor: Bulldozer FX-6100/8100 (athlon64)
Jbigi: Locally optimized native BigInteger library loaded from file
Encoding: UTF-8
Charset: UTF-8
Version	0.4.1
Signed by	str4d@mail.i2p
Date	2015-09-12 15:46
Author	str4d@mail.i2p
11/03/15 14:03:28.565 ERROR [EmailChecker] i2p.bote.service.EmailChecker : Error while checking whether new mail has arrived.
     java.util.concurrent.ExecutionException: java.util.concurrent.ExecutionException: java.util.ConcurrentModificationException
     at java.util.concurrent.FutureTask.get(
     at i2p.bote.service.EmailChecker.updatePendingTasks(
     at i2p.bote.service.EmailChecker.isCheckingForMail(
     at i2p.bote.service.EmailChecker.checkForMail(
     Caused by: java.util.concurrent.ExecutionException: java.util.ConcurrentModificationException
     at java.util.concurrent.FutureTask.get(
     at java.util.concurrent.ThreadPoolExecutor.runWorker(
     at java.util.concurrent.ThreadPoolExecutor$
     Caused by: java.util.ConcurrentModificationException
     at java.util.ArrayList$Itr.checkForComodification(
     at java.util.ArrayList$
     at java.util.concurrent.Executors$
     ... 4 more


Change History (4)

comment:1 Changed 6 years ago by zzz

Owner: set to str4d
Status: newassigned
Version: 0.9.22

On quick glance, the locking in BucketManager? looks shaky. See also previously fixed #1586 (or was it…)

May be desirable to someday migrate to common net.i2p.kademlia, although there may be features in HH's impl we need to backport first. Last I looked, his had some advantages. Also the whole SBucket thing… Anyway, just musing, irrelevant to this ticket.

comment:2 Changed 6 years ago by kay

It seems (here and in the other ticket mentioned above) that
BucketManager?.getAllUnlockedPeers iterates over a bucket peer-list
that gets changed during the iteration. The List of peers is
a Collections.synchronizedList that mandates manual synchronization
when iterating over it.

I propose two solutions both concerning the AbstractBucket? class:

1) use CopyOnWriteArrayList? for the peers' List, i.e.

 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
@@ -46,7 +49,7 @@ abstract class AbstractBucket implements
     protected int capacity;
     public AbstractBucket(int capacity) {
-        peers = Collections.synchronizedList(new ArrayList<KademliaPeer>());
+        peers = new CopyOnWriteArrayList<KademliaPeer>();
         this.capacity = capacity;

2) return a copy-iterator, i.e. using i2p.bote.Util

@@ -109,5 +114,6 @@ abstract class AbstractBucket implements
     public Iterator<KademliaPeer> iterator() {
-        return peers.iterator();
+        return Util.synchronizedCopy(peers).iterator();

Solution no. 1 seems better b/c the iterator would throw
an UnsupportedOperationException? for modification attempts,
and thus fail fast.

comment:3 Changed 6 years ago by str4d

Milestone: undecided0.9.23
Status: assignedtesting

I concur with solution 1, it's the same thing I did for BucketManager? in #1586.

Committed in f9edfbef5dfd2aa7109ff5761992774a6a82c249.

comment:4 Changed 6 years ago by str4d

Resolution: fixed
Status: testingclosed

Haven't had any more reports after 0.4.2 release, assuming fixed.

Note: See TracTickets for help on using tickets.