Toad fix burst only (old revived: needs careful review!)

src/freenet/clients/http/DarknetConnectionsToadlet.java

@@ -401,7 +401,7 @@ protected void handleAltPost(URI uri, HTTPRequest request, ToadletContext ctx, b
 			DarknetPeerNode[] peerNodes = node.getDarknetConnections();
 			for(DarknetPeerNode pn: peerNodes) {
 				if (request.isPartSet("node_"+pn.hashCode())) {	
-					if((pn.timeLastConnectionCompleted() < (System.currentTimeMillis() - 1000*60*60*24*7) /* one week */) ||  (pn.peerNodeStatus == PeerManager.PEER_NODE_STATUS_NEVER_CONNECTED) || request.isPartSet("forceit")){
+					if((pn.timeLastConnectionCompleted() < (System.currentTimeMillis() - 1000*60*60*24*7) /* one week */) ||  (pn.getPeerNodeStatus() == PeerManager.PEER_NODE_STATUS_NEVER_CONNECTED) || request.isPartSet("forceit")){
 						this.node.removePeerConnection(pn);
 						if(logMINOR) Logger.minor(this, "Removed node: node_"+pn.hashCode());
 					}else{

src/freenet/node/CHKInsertHandler.java

@@ -444,7 +444,22 @@ private void finish(int code) {
         			// Ignore.
         		}
 
-        		Logger.error(this, "Insert took too long, telling downstream that it's finished and reassigning to self on "+this);
+        		/* Our predecessor expects us to timeout within the fixed timeout time. However
+        		 * it takes some time to forward the request, and we might have sent it to several
+        		 * nodes, so it's possible that CHKInsertSender is still waiting for downstream
+        		 * nodes to reply even though our predecessor has timed out. This will happen any
+        		 * time the last node fails to respond.
+        		 * 
+        		 * Also, for load management purposes, it is important to know exactly how many
+        		 * of our requests are running, regardless of timeouts (especially with NLM).
+        		 * 
+        		 * The solution implemented here is to take ownership of the insert ourself and
+        		 * tell our predecessor that we've timed out.
+        		 * 
+        		 * FIXME Reconsider.
+        		 * 
+        		 * See comments on TRANSFER_COMPLETION_ACK_TIMEOUT_BULK etc. */
+        		Logger.normal(this, "Insert took too long, telling downstream that it's finished and reassigning to self on "+this);
 
         		// Still waiting.
         		while(true) {

src/freenet/node/CHKInsertSender.java

@@ -340,7 +340,15 @@ void start() {
 	}
 
     // Constants
+	/* Time for a node to accept an insert */
     static final long ACCEPTED_TIMEOUT = SECONDS.toMillis(10);
+    /* We have a fixed timeout. This causes some problems, since if the last node on the
+     * chain fails, the first node to reach its timeout will be the originator (htl 18),
+     * because it started counting first! In 0.5 the timeout depended on the HTL, but the
+     * HTL does not strictly decrease (we spend several hops at HTL 18 and HTL 1). 
+     * FIXME Reconsider.
+     * 
+     * There are related complexities in CHKInsertHandler. */
     static final long TRANSFER_COMPLETION_ACK_TIMEOUT_REALTIME = MINUTES.toMillis(1);
     static final long TRANSFER_COMPLETION_ACK_TIMEOUT_BULK = MINUTES.toMillis(5);
 

src/freenet/node/DarknetPeerNode.java

@@ -300,11 +300,11 @@ public synchronized String getName() {
 	}
 
 	@Override
-	protected synchronized int getPeerNodeStatus(long now, long backedOffUntilRT, long backedOffUntilBulk, boolean overPingThreshold, boolean noLoadStats) {
+	protected synchronized int calculatePeerNodeStatus(long now, long backedOffUntilRT, long backedOffUntilBulk, boolean overPingThreshold, boolean noLoadStats) {
 		if(isDisabled) {
 			return PeerManager.PEER_NODE_STATUS_DISABLED;
 		}
-		int status = super.getPeerNodeStatus(now, backedOffUntilRT, backedOffUntilBulk, overPingThreshold, noLoadStats);
+		int status = super.calculatePeerNodeStatus(now, backedOffUntilRT, backedOffUntilBulk, overPingThreshold, noLoadStats);
 		if(status == PeerManager.PEER_NODE_STATUS_CONNECTED ||
 				status == PeerManager.PEER_NODE_STATUS_CLOCK_PROBLEM ||
 				status == PeerManager.PEER_NODE_STATUS_ROUTING_BACKED_OFF ||

src/freenet/node/NodeCrypto.java

@@ -568,6 +568,7 @@ public void maybeBootConnection(PeerNode peerNode,
 		for(PeerNode pn : possibleMatches) {
 			if(pn == peerNode) continue;
 			if(pn.equals(peerNode)) continue;
+			if(logMINOR) Logger.minor(this, "Booting connection "+peerNode+" because have "+pn+" on same IP");
 			if(pn.crypto.config.oneConnectionPerAddress()) {
 				if(pn instanceof DarknetPeerNode) {
 					if(!(peerNode instanceof DarknetPeerNode)) {

src/freenet/node/PeerManager.java

@@ -335,8 +335,8 @@ public boolean addPeer(PeerNode pn) {
 	 */
 	boolean addPeer(PeerNode pn, boolean ignoreOpennet, boolean reactivate) {
 		assert (pn != null);
-		if(reactivate)
-			pn.forceCancelDisconnecting();
+		// Call onAdded() first for the case where it is present but disconnecting.
+        pn.onAdded();
 		synchronized(this) {
 			for(PeerNode myPeer: myPeers) {
 				if(myPeer.equals(pn)) {
@@ -349,9 +349,6 @@ boolean addPeer(PeerNode pn, boolean ignoreOpennet, boolean reactivate) {
 			myPeers[myPeers.length - 1] = pn;
 			Logger.normal(this, "Added " + pn);
 		}
-		if(pn.recordStatus())
-			addPeerNodeStatus(pn.getPeerNodeStatus(), pn, false);
-		pn.setPeerNodeStatus(System.currentTimeMillis());
 		if((!ignoreOpennet) && pn instanceof OpennetPeerNode) {
 			OpennetManager opennet = node.getOpennet();
 			if(opennet != null)

src/freenet/node/PeerNode.java

@@ -307,7 +307,7 @@ public abstract class PeerNode implements USKRetrieverCallback, BasePeerNode, Pe
 	/** The time at which we last completed a connection setup. */
 	private long connectedTime;
 	/** The status of this peer node in terms of Node.PEER_NODE_STATUS_* */
-	public int peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED;
+	private int peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED;
 
 	static final long CHECK_FOR_SWAPPED_TRACKERS_INTERVAL = FNPPacketMangler.SESSION_KEY_REKEYING_INTERVAL / 30;
 
@@ -423,6 +423,8 @@ protected boolean ignoreLastGoodVersion() {
 	* because it hasn't been connected since the last major crypto change. */
 	public PeerNode(SimpleFieldSet fs, Node node2, NodeCrypto crypto, boolean fromLocal) 
 	                throws FSParseException, PeerParseException, ReferenceSignatureVerificationException, PeerTooOldException {
+
+	    removed = true;
 		boolean noSig = false;
 		if(fromLocal || fromAnonymousInitiator()) noSig = true;
 		myRef = new WeakReference<PeerNode>(this);
@@ -1412,19 +1414,21 @@ public long lastSentPacketTime() {
 	public boolean shouldSendHandshake() {
 		long now = System.currentTimeMillis();
 		boolean tempShouldSendHandshake = false;
+		boolean disconnected;
 		synchronized(this) {
 			if(disconnecting) return false;
-			tempShouldSendHandshake = ((now > sendHandshakeTime) && (handshakeIPs != null) && (isRekeying || !isConnected()));
+			disconnected = !isConnected();
+			tempShouldSendHandshake = ((now > sendHandshakeTime) && (handshakeIPs != null) && (isRekeying || disconnected));
 		}
 		if(logMINOR) Logger.minor(this, "shouldSendHandshake(): initial = "+tempShouldSendHandshake);
 		if(tempShouldSendHandshake && (hasLiveHandshake(now)))
 			tempShouldSendHandshake = false;
 		if(tempShouldSendHandshake) {
-			if(isBurstOnly()) {
+			if(disconnected && isBurstOnly()) {
 				synchronized(this) {
 					isBursting = true;
 				}
-				setPeerNodeStatus(System.currentTimeMillis());
+			    setPeerNodeStatus(System.currentTimeMillis());
 			} else
 				return true;
 		}
@@ -1461,7 +1465,7 @@ public boolean hasLiveHandshake(long now) {
 	 * Set sendHandshakeTime, and return whether to fetch the ARK.
 	 */
 	protected boolean innerCalcNextHandshake(boolean successfulHandshakeSend, boolean dontFetchARK, long now) {
-		if(isBurstOnly())
+		if((!isConnected()) && isBurstOnly())
 			return calcNextHandshakeBurstOnly(now);
 		synchronized(this) {
 			long delay;
@@ -1744,7 +1748,10 @@ public void updateLocation(double newLoc, double[] newLocs) {
 		node.getPeers().updatePMUserAlert();
 		if(anythingChanged)
 		    writePeers();
-		setPeerNodeStatus(System.currentTimeMillis());
+		synchronized(this) {
+		    if(!isConnected()) return; // Race condition.
+	        setPeerNodeStatus(System.currentTimeMillis());
+		}
 	}
 
 	/** Write the peers list affecting this node. */
@@ -3397,9 +3404,16 @@ else if(status == PeerManager.PEER_NODE_STATUS_NO_LOAD_STATS)
 			return "peer_unknown_status";
 	}
 
-	protected synchronized int getPeerNodeStatus(long now, long routingBackedOffUntilRT, long localRoutingBackedOffUntilBulk, boolean overPingTime, boolean noLoadStats) {
+	/** Calculate the current status. Does not update peerNodeStatus itself, nor does it update
+	 * the various listeners and trackers interested in peer statuses; see 
+	 * setPeerNodeStatus(long, boolean) for that. Override this method to implement special peer
+	 * statuses that are specific to your PeerNode implementation (e.g. DarknetPeerNode).
+	 * You should generally call this method and then adjust the return value in special cases.
+	 * Note that this method updates the backoff statistics. */
+	protected synchronized int calculatePeerNodeStatus(long now, long routingBackedOffUntilRT, long localRoutingBackedOffUntilBulk, boolean overPingTime, boolean noLoadStats) {
 		if(disconnecting)
 			return PeerManager.PEER_NODE_STATUS_DISCONNECTING;
+		int peerNodeStatus; // Caller's responsibility to update this.peerNodeStatus.
 		boolean isConnected = isConnected();
 		if(isRoutable()) {  // Function use also updates timeLastConnected and timeLastRoutable
 			if(noLoadStats)
@@ -3460,13 +3474,15 @@ else if(isBursting)
 			return PeerManager.PEER_NODE_STATUS_BURSTING;
 		else
 			peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED;
-		if(!isConnected && (previousRoutingBackoffReasonRT != null)) {
-			peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonRT, this, true);
-			previousRoutingBackoffReasonRT = null;
-		}
-		if(!isConnected && (previousRoutingBackoffReasonBulk != null)) {
-			peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonBulk, this, false);
-			previousRoutingBackoffReasonBulk = null;
+		if(!removed) {
+		    if(!isConnected && (previousRoutingBackoffReasonRT != null)) {
+		        peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonRT, this, true);
+		        previousRoutingBackoffReasonRT = null;
+		    }
+		    if(!isConnected && (previousRoutingBackoffReasonBulk != null)) {
+		        peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonBulk, this, false);
+		        previousRoutingBackoffReasonBulk = null;
+		    }
 		}
 		return peerNodeStatus;
 	}
@@ -3475,17 +3491,22 @@ public int setPeerNodeStatus(long now) {
 		return setPeerNodeStatus(now, false);
 	}
 
-	public int setPeerNodeStatus(long now, boolean noLog) {
+	/* Calls calculatePeerNodeStatus() to calculate the current status, and then updates everyone
+	 * who needs to know. In particular the PeerStatusTracker but also load management stuff and
+	 * UI listeners. Should not be overridden in general. 
+	 * @param now The current time.
+	 * @param noLog If true don't complain if a peer status e.g. isn't in the tracker. */
+	public final int setPeerNodeStatus(long now, boolean noLog) {
 		long localRoutingBackedOffUntilRT = getRoutingBackedOffUntil(true);
 		long localRoutingBackedOffUntilBulk = getRoutingBackedOffUntil(true);
 		int oldPeerNodeStatus;
 		long threshold = maxPeerPingTime();
 		boolean noLoadStats = noLoadStats();
 		synchronized(this) {
 			oldPeerNodeStatus = peerNodeStatus;
-			peerNodeStatus = getPeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats);
+			peerNodeStatus = calculatePeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats);
 
-			if(peerNodeStatus != oldPeerNodeStatus && recordStatus()) {
+			if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !removed) {
 				peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog);
 			}
 
@@ -3835,11 +3856,9 @@ public boolean notifyDisconnecting(boolean dumpMessageQueue) {
 
 	/** Called to cancel a delayed disconnect. Always succeeds even if the node was not being
 	 * disconnected. */
-	public void forceCancelDisconnecting() {
+	public void onAdded() {
 		synchronized(this) {
 			removed = false;
-			if(!disconnecting)
-				return;
 			disconnecting = false;
 		}
 		setPeerNodeStatus(System.currentTimeMillis(), true);

src/freenet/node/PeerStatusTracker.java

@@ -11,7 +11,7 @@ class PeerStatusTracker<K extends Object> {
 
     private static volatile boolean logMINOR;
     static {
-        Logger.registerClass(PeerManager.class);
+        Logger.registerClass(PeerStatusTracker.class);
     }
 
 	/** PeerNode statuses, by status. WARNING: LOCK THIS LAST. Must NOT call PeerNode inside this lock. */
@@ -29,13 +29,13 @@ public synchronized void addStatus(K peerNodeStatus, PeerNode peerNode, boolean
 					Logger.error(this, "addPeerNodeStatus(): node already in peerNodeStatuses: " + peerNode + " status " + peerNodeStatus, new Exception("debug"));
 				return;
 			}
-			statuses.remove(peerNodeStatus);
-		} else
+		} else {
 			statusSet = new WeakHashSet<PeerNode>();
+			statuses.put(peerNodeStatus, statusSet);
+		}
 		if(logMINOR)
-			Logger.minor(this, "addPeerNodeStatus(): adding PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'");
+			Logger.minor(this, "addPeerNodeStatus(): adding PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'", new Exception("debug"));
 		statusSet.add(peerNode);
-		statuses.put(peerNodeStatus, statusSet);
 	}
 
 	public synchronized int statusSize(K pnStatus) {
@@ -59,7 +59,7 @@ public synchronized void removeStatus(K peerNodeStatus, PeerNode peerNode,
 				statuses.remove(peerNodeStatus);
 		}
 		if(logMINOR)
-			Logger.minor(this, "removePeerNodeStatus(): removing PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'");
+			Logger.minor(this, "removePeerNodeStatus(): removing PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'", new Exception("debug"));
 	}
 
 	public synchronized void changePeerNodeStatus(PeerNode peerNode, K oldPeerNodeStatus,