Fix burst only

src/freenet/clients/http/DarknetConnectionsToadlet.java

@@ -394,7 +394,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/Node.java

@@ -4416,7 +4416,7 @@ public int getUnclaimedFIFOSize() {
 	 * Connect this node to another node (for purposes of testing)
 	 */
 	public void connectToSeednode(SeedServerTestPeerNode node) throws OpennetDisabledException, FSParseException, PeerParseException, ReferenceSignatureVerificationException {
-		peers.addPeer(node,false,false);
+		peers.addPeer(node,false);
 	}
 	public void connect(Node node, FRIEND_TRUST trust, FRIEND_VISIBILITY visibility) throws FSParseException, PeerParseException, ReferenceSignatureVerificationException {
 		peers.connect(node.darknetCrypto.exportPublicFieldSet(), darknetCrypto.packetMangler, trust, visibility);

src/freenet/node/NodeCrypto.java

@@ -592,6 +592,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/OpennetManager.java

@@ -612,7 +612,7 @@ public boolean wantPeer(OpennetPeerNode nodeToAddNow, boolean addAtLRU, boolean
 			nodeToAddNow.setAddedReason(connectionType);
 		if(notMany) {
 			if(nodeToAddNow != null) {
-				node.peers.addPeer(nodeToAddNow, true, true); // Add to peers outside the OM lock
+				node.peers.addPeer(nodeToAddNow, true); // Add to peers outside the OM lock
 			}
 			return true;
 		}
@@ -691,7 +691,7 @@ public boolean wantPeer(OpennetPeerNode nodeToAddNow, boolean addAtLRU, boolean
 				}
 			}
 		}
-		if(nodeToAddNow != null && canAdd && !node.peers.addPeer(nodeToAddNow, true, true)) {
+		if(nodeToAddNow != null && canAdd && !node.peers.addPeer(nodeToAddNow, true)) {
 			if(logMINOR)
 				Logger.minor(this, "Already in global peers list: "+nodeToAddNow+" when adding opennet node");
 			// Just because it's in the global peers list doesn't mean its in the LRU, it may be an old-opennet-peers reconnection.

src/freenet/node/PeerManager.java

@@ -252,7 +252,7 @@ private boolean readPeers(File peersFile, OutgoingPacketMangler mangler, NodeCry
 					    else
 					        opennet.addOldOpennetNode((OpennetPeerNode)pn);
 					} else
-						addPeer(pn, true, false);
+						addPeer(pn, true);
 					gotSome = true;
 				} catch(FSParseException e2) {
 					Logger.error(this, "Could not parse peer: " + e2 + '\n' + fs.toString(), e2);
@@ -305,7 +305,7 @@ private boolean readPeers(File peersFile, OutgoingPacketMangler mangler, NodeCry
 	}
 
 	public boolean addPeer(PeerNode pn) {
-		return addPeer(pn, false, false);
+		return addPeer(pn, false);
 	}
 
 	/**
@@ -317,10 +317,10 @@ public boolean addPeer(PeerNode pn) {
 	 * @return True if the node was successfully added. False if it was already present, or if we tried to add
 	 * an opennet peer when opennet was disabled.
 	 */
-	boolean addPeer(PeerNode pn, boolean ignoreOpennet, boolean reactivate) {
+	boolean addPeer(PeerNode pn, boolean ignoreOpennet) {
 		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)) {
@@ -333,9 +333,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)
@@ -1800,11 +1797,9 @@ public void maybeLogPeerNodeStatusSummary(long now) {
 
 	public void changePeerNodeStatus(PeerNode peerNode, int oldPeerNodeStatus,
 			int peerNodeStatus, boolean noLog) {
-		Integer newStatus = Integer.valueOf(peerNodeStatus);
-		Integer oldStatus = Integer.valueOf(oldPeerNodeStatus);
-		this.allPeersStatuses.changePeerNodeStatus(peerNode, oldStatus, newStatus, noLog);
+		this.allPeersStatuses.changePeerNodeStatus(peerNode, oldPeerNodeStatus, peerNodeStatus, noLog);
 		if(!peerNode.isOpennet())
-			this.darknetPeersStatuses.changePeerNodeStatus(peerNode, oldStatus, newStatus, noLog);
+			this.darknetPeersStatuses.changePeerNodeStatus(peerNode, oldPeerNodeStatus, peerNodeStatus, noLog);
 		node.executor.execute(new Runnable() {
 
 			@Override

src/freenet/node/PeerNode.java

@@ -320,7 +320,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;
 
@@ -453,6 +453,7 @@ protected boolean ignoreLastGoodVersion() {
 	* @param node2 The running Node we are part of.
 	*/
 	public PeerNode(SimpleFieldSet fs, Node node2, NodeCrypto crypto, PeerManager peers, boolean fromLocal, boolean fromAnonymousInitiator, OutgoingPacketMangler mangler, boolean isOpennet) throws FSParseException, PeerParseException, ReferenceSignatureVerificationException {
+	    removed = true;
 		boolean noSig = false;
 		if(fromLocal || fromAnonymousInitiator) noSig = true;
 		myRef = new WeakReference<PeerNode>(this);
@@ -1490,19 +1491,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;
 		}
@@ -1539,7 +1542,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;
@@ -1824,7 +1827,10 @@ public void updateLocation(double newLoc, double[] newLocs) {
 			// Not urgent. This makes up the majority of the total writes.
 			// Writing it on shutdown is sufficient.
 			node.peers.writePeers(isOpennet());
-		setPeerNodeStatus(System.currentTimeMillis());
+		synchronized(this) {
+		    if(!isConnected()) return; // Race condition.
+	        setPeerNodeStatus(System.currentTimeMillis());
+		}
 	}
 
 	/**
@@ -3480,9 +3486,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)
@@ -3543,13 +3556,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;
 	}
@@ -3558,17 +3573,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);
 			}
 
@@ -3935,11 +3955,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,