Author: toad
Date: 2006-07-12 00:50:28 +0000 (Wed, 12 Jul 2006)
New Revision: 9572
Modified:
trunk/freenet/src/freenet/client/async/ClientGetter.java
trunk/freenet/src/freenet/node/ARKFetcher.java
trunk/freenet/src/freenet/node/PeerNode.java
trunk/freenet/src/freenet/node/Version.java
Log:
876: Nested locks considered harmful!
Modified: trunk/freenet/src/freenet/client/async/ClientGetter.java
===================================================================
--- trunk/freenet/src/freenet/client/async/ClientGetter.java 2006-07-12
00:27:09 UTC (rev 9571)
+++ trunk/freenet/src/freenet/client/async/ClientGetter.java 2006-07-12
00:50:28 UTC (rev 9572)
@@ -53,11 +53,16 @@
archiveRestarts = 0;
}
- public synchronized void start() throws FetchException {
+ public void start() throws FetchException {
try {
- currentState = SingleFileFetcher.create(this, this, new
ClientMetadata(),
- uri, ctx, actx,
ctx.maxNonSplitfileRetries, 0, false, null, true,
- returnBucket);
+ // FIXME synchronization is probably unnecessary.
+ // But we DEFINITELY do not want to synchronize while
calling currentState.schedule(),
+ // which can call onSuccess and thereby almost anything.
+ synchronized(this) {
+ currentState = SingleFileFetcher.create(this,
this, new ClientMetadata(),
+ uri, ctx, actx,
ctx.maxNonSplitfileRetries, 0, false, null, true,
+ returnBucket);
+ }
if(currentState != null)
currentState.schedule();
} catch (MalformedURLException e) {
@@ -65,9 +70,16 @@
}
}
- public synchronized void onSuccess(FetchResult result, ClientGetState
state) {
- finished = true;
- currentState = null;
+ public void onSuccess(FetchResult result, ClientGetState state) {
+ synchronized(this) {
+ finished = true;
+ currentState = null;
+ }
+ // Rest of method does not need to be synchronized.
+ // Variables will be updated on exit of method, and the only
thing that is
+ // set is the returnBucket and the result. Not locking not only
prevents
+ // nested locking resulting in deadlocks, it also prevents long
locks due to
+ // doing massive encrypted I/Os while holding a lock.
if((returnBucket != null) && (result.asBucket() !=
returnBucket)) {
Bucket from = result.asBucket();
Bucket to = returnBucket;
Modified: trunk/freenet/src/freenet/node/ARKFetcher.java
===================================================================
--- trunk/freenet/src/freenet/node/ARKFetcher.java 2006-07-12 00:27:09 UTC
(rev 9571)
+++ trunk/freenet/src/freenet/node/ARKFetcher.java 2006-07-12 00:50:28 UTC
(rev 9572)
@@ -62,40 +62,42 @@
/**
* Called when the ARKFetchManager says it's our turn to start fetching.
*/
- public synchronized void start() {
+ public void start() {
if(node.arkFetchManager.hasReadyARKFetcher(this)) {
node.arkFetchManager.removeReadyARKFetcher(this);
}
if(peer.isConnected()) {
return;
}
- if(isFetching) {
- return;
- }
ClientGetter cg = null;
- if(started) { // We only need one ARKFetcher per PeerNode
- return;
- }
- Logger.minor(this, "Starting ... for "+peer+" on "+this);
- Logger.normal( this, "Starting ARK Fetcher after
"+peer.getHandshakeCount()+" failed handshakes for "+peer.getPeer()+" with
identity '"+peer.getIdentityString()+"'");
- started = true;
- // Start fetch
- shouldRun = true;
- if(getter == null) {
- USK ark = peer.getARK();
- if(ark == null) {
+ synchronized(this) {
+ if(isFetching) {
return;
}
- FreenetURI uri = ark.getURI();
- startedEdition = uri.getSuggestedEdition();
- fetchingURI = uri;
- Logger.minor(this, "Fetching ARK: "+uri+" for "+peer);
- cg = new ClientGetter(this, node.chkFetchScheduler,
node.sskFetchScheduler,
- uri, node.arkFetcherContext,
RequestStarter.UPDATE_PRIORITY_CLASS,
- this, new ArrayBucket());
- getter = cg;
- } else return; // already running
-
+ if(started) { // We only need one ARKFetcher per
PeerNode
+ return;
+ }
+ Logger.minor(this, "Starting ... for "+peer+" on
"+this);
+ Logger.normal( this, "Starting ARK Fetcher after
"+peer.getHandshakeCount()+" failed handshakes for "+peer.getPeer()+" with
identity '"+peer.getIdentityString()+"'");
+ started = true;
+ // Start fetch
+ shouldRun = true;
+ if(getter == null) {
+ USK ark = peer.getARK();
+ if(ark == null) {
+ return;
+ }
+ FreenetURI uri = ark.getURI();
+ startedEdition = uri.getSuggestedEdition();
+ fetchingURI = uri;
+ Logger.minor(this, "Fetching ARK: "+uri+" for
"+peer);
+ cg = new ClientGetter(this,
node.chkFetchScheduler, node.sskFetchScheduler,
+ uri, node.arkFetcherContext,
RequestStarter.UPDATE_PRIORITY_CLASS,
+ this, new ArrayBucket());
+ getter = cg;
+ } else return; // already running
+ }
+
if(cg != null)
try {
if(!isFetching) {
Modified: trunk/freenet/src/freenet/node/PeerNode.java
===================================================================
--- trunk/freenet/src/freenet/node/PeerNode.java 2006-07-12 00:27:09 UTC
(rev 9571)
+++ trunk/freenet/src/freenet/node/PeerNode.java 2006-07-12 00:50:28 UTC
(rev 9572)
@@ -1315,8 +1315,15 @@
/**
* Process a new nodereference, as a SimpleFieldSet.
*/
- private synchronized void processNewNoderef(SimpleFieldSet fs, boolean
forARK) throws FSParseException {
+ private void processNewNoderef(SimpleFieldSet fs, boolean forARK) throws
FSParseException {
Logger.minor(this, "Parsing: \n"+fs);
+ boolean changedAnything = innerProcessNewNoderef(fs, forARK);
+ if(changedAnything) node.peers.writePeers();
+ }
+
+ /** The synchronized part of processNewNoderef
+ * @throws FSParseException */
+ private synchronized boolean innerProcessNewNoderef(SimpleFieldSet fs,
boolean forARK) throws FSParseException {
boolean changedAnything = false;
String identityString = fs.get("identity");
try {
@@ -1402,10 +1409,11 @@
changedAnything = true;
if(!name.equals(myName)) changedAnything = true;
myName = name;
- if(changedAnything) node.peers.writePeers();
- }
+
+ return changedAnything;
+ }
- /**
+ /**
* Send a payload-less packet on either key if necessary.
* @throws PacketSequenceException If there is an error sending the packet
* caused by a sequence inconsistency.
@@ -2026,10 +2034,10 @@
}
public void disablePeer() {
+ isDisabled = true;
if(isConnected) {
forceDisconnect();
}
- isDisabled = true;
arkFetcher.stop();
setPeerNodeStatus(System.currentTimeMillis());
node.peers.writePeers();
Modified: trunk/freenet/src/freenet/node/Version.java
===================================================================
--- trunk/freenet/src/freenet/node/Version.java 2006-07-12 00:27:09 UTC (rev
9571)
+++ trunk/freenet/src/freenet/node/Version.java 2006-07-12 00:50:28 UTC (rev
9572)
@@ -18,7 +18,7 @@
public static final String protocolVersion = "1.0";
/** The build number of the current revision */
- private static final int buildNumber = 875;
+ private static final int buildNumber = 876;
/** Oldest build of Fred we will talk to */
private static final int oldLastGoodBuild = 844;
