From 3d3352438b88d8c6a813cf4e302cbdef7b2f6a3f Mon Sep 17 00:00:00 2001 From: zzz Date: Fri, 2 Sep 2011 17:25:26 +0000 Subject: [PATCH] * NetDB: Hopefully fix ISJ deadlock, thx dream --- history.txt | 4 +++ .../src/net/i2p/router/RouterVersion.java | 2 +- .../kademlia/FloodOnlySearchJob.java | 13 +++---- .../networkdb/kademlia/FloodSearchJob.java | 35 +++++++++++++------ .../kademlia/IterativeSearchJob.java | 14 ++++---- 5 files changed, 41 insertions(+), 27 deletions(-) diff --git a/history.txt b/history.txt index 153b56e0a..b8faa1964 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,7 @@ +2011-09-02 zzz + * Console: Cache user-agent processing + * NetDB: Hopefully fix ISJ deadlock, thx dream + 2011-09-02 sponge * I2PSnark: Fix GUI html tag for adding a torrent, it was missing a space. diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index a6204817e..f2522cd4e 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -18,7 +18,7 @@ public class RouterVersion { /** deprecated */ public final static String ID = "Monotone"; public final static String VERSION = CoreVersion.VERSION; - public final static long BUILD = 8; + public final static long BUILD = 9; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlySearchJob.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlySearchJob.java index ac6f3c5c6..505417701 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlySearchJob.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/FloodOnlySearchJob.java @@ -221,12 +221,10 @@ class FloodOnlySearchJob extends FloodSearchJob { } _facade.complete(_key); getContext().statManager().addRateData("netDb.failedTime", time, 0); - synchronized (_onFailed) { - for (int i = 0; i < _onFailed.size(); i++) { - Job j = _onFailed.remove(0); - getContext().jobQueue().addJob(j); - } + for (Job j : _onFailed) { + getContext().jobQueue().addJob(j); } + _onFailed.clear(); } @Override @@ -254,9 +252,8 @@ class FloodOnlySearchJob extends FloodSearchJob { } _facade.complete(_key); getContext().statManager().addRateData("netDb.successTime", time, 0); - synchronized (_onFind) { - while (!_onFind.isEmpty()) - getContext().jobQueue().addJob(_onFind.remove(0)); + for (Job j : _onFind) { + getContext().jobQueue().addJob(j); } } } diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodSearchJob.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodSearchJob.java index 261687deb..fce3fbd63 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/FloodSearchJob.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/FloodSearchJob.java @@ -1,7 +1,7 @@ package net.i2p.router.networkdb.kademlia; -import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import net.i2p.data.Hash; import net.i2p.data.i2np.DatabaseLookupMessage; @@ -41,15 +41,21 @@ public class FloodSearchJob extends JobImpl { protected volatile boolean _dead; protected final long _created; + /** + * @param onFind may be null + * @param onFailed may be null + */ public FloodSearchJob(RouterContext ctx, FloodfillNetworkDatabaseFacade facade, Hash key, Job onFind, Job onFailed, int timeoutMs, boolean isLease) { super(ctx); _log = ctx.logManager().getLog(getClass()); _facade = facade; _key = key; - _onFind = new ArrayList(4); - _onFind.add(onFind); - _onFailed = new ArrayList(4); - _onFailed.add(onFailed); + _onFind = new CopyOnWriteArrayList(); + if (onFind != null) + _onFind.add(onFind); + _onFailed = new CopyOnWriteArrayList(); + if (onFailed != null) + _onFailed.add(onFailed); int timeout = timeoutMs / FLOOD_SEARCH_TIME_FACTOR; if (timeout < timeoutMs) timeout = timeoutMs; @@ -64,14 +70,23 @@ public class FloodSearchJob extends JobImpl { /** * Add jobs to an existing search + * @param onFind may be null + * @param onFailed may be null + * @param timeoutMs ignored + * @param isLease ignored */ void addDeferred(Job onFind, Job onFailed, long timeoutMs, boolean isLease) { - if (_dead) { - getContext().jobQueue().addJob(onFailed); - } else { - if (onFind != null) synchronized (_onFind) { _onFind.add(onFind); } - if (onFailed != null) synchronized (_onFailed) { _onFailed.add(onFailed); } + synchronized (this) { + if (!_dead) { + if (onFind != null) + _onFind.add(onFind); + if (onFailed != null) + _onFailed.add(onFailed); + return; + } } + // outside synch to avoid deadlock with job queue + getContext().jobQueue().addJob(onFailed); } /** using context clock */ diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/IterativeSearchJob.java b/router/java/src/net/i2p/router/networkdb/kademlia/IterativeSearchJob.java index 20006f0d8..8fabbdf27 100644 --- a/router/java/src/net/i2p/router/networkdb/kademlia/IterativeSearchJob.java +++ b/router/java/src/net/i2p/router/networkdb/kademlia/IterativeSearchJob.java @@ -320,12 +320,10 @@ class IterativeSearchJob extends FloodSearchJob { } getContext().statManager().addRateData("netDb.failedTime", time, 0); getContext().statManager().addRateData("netDb.retries", Math.max(0, tries - 1), 0); - synchronized (_onFailed) { - for (int i = 0; i < _onFailed.size(); i++) { - Job j = _onFailed.remove(0); - getContext().jobQueue().addJob(j); - } + for (Job j : _onFailed) { + getContext().jobQueue().addJob(j); } + _onFailed.clear(); } @Override @@ -356,9 +354,9 @@ class IterativeSearchJob extends FloodSearchJob { ", peers queried: " + tries); getContext().statManager().addRateData("netDb.successTime", time, 0); getContext().statManager().addRateData("netDb.retries", tries - 1, 0); - synchronized (_onFind) { - while (!_onFind.isEmpty()) - getContext().jobQueue().addJob(_onFind.remove(0)); + for (Job j : _onFind) { + getContext().jobQueue().addJob(j); } + _onFind.clear(); } }