Opened 5 years ago
Closed 5 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 |
Description
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.report(FutureTask.java:122) at java.util.concurrent.FutureTask.get(FutureTask.java:206) at i2p.bote.service.EmailChecker.updatePendingTasks(EmailChecker.java:202) at i2p.bote.service.EmailChecker.isCheckingForMail(EmailChecker.java:152) at i2p.bote.service.EmailChecker.checkForMail(EmailChecker.java:112) at i2p.bote.service.EmailChecker.run(EmailChecker.java:230) Caused by: java.util.concurrent.ExecutionException: java.util.ConcurrentModificationException at java.util.concurrent.FutureTask.report(FutureTask.java:122) at java.util.concurrent.FutureTask.get(FutureTask.java:206) at i2p.bote.network.CheckEmailTask.call(CheckEmailTask.java:131) at i2p.bote.network.CheckEmailTask.call(CheckEmailTask.java:61) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745) Caused by: java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901) at java.util.ArrayList$Itr.next(ArrayList.java:851) at i2p.bote.network.kademlia.BucketManager.getAllUnlockedPeers(BucketManager.java:268) at i2p.bote.network.kademlia.ClosestNodesLookupTask.call(ClosestNodesLookupTask.java:107) at i2p.bote.network.kademlia.KademliaDHT.getClosestNodes(KademliaDHT.java:180) at i2p.bote.network.kademlia.KademliaDHT.find(KademliaDHT.java:263) at i2p.bote.network.kademlia.KademliaDHT.findAll(KademliaDHT.java:190) at i2p.bote.network.CheckEmailTask$EmailPacketTask.run(CheckEmailTask.java:201) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) ... 4 more
Subtickets
Change History (4)
comment:1 Changed 5 years ago by
Owner: | set to str4d |
---|---|
Status: | new → assigned |
Version: | → 0.9.22 |
comment:2 Changed 5 years ago by
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; import net.i2p.data.Destination; import net.i2p.data.Hash; @@ -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 @Override 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 5 years ago by
Milestone: | undecided → 0.9.23 |
---|---|
Status: | assigned → testing |
I concur with solution 1, it's the same thing I did for BucketManager? in #1586.
Committed in f9edfbef5dfd2aa7109ff5761992774a6a82c249
.
comment:4 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | testing → closed |
Haven't had any more reports after 0.4.2 release, assuming fixed.
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.