From 917db6546e3ee2ebddd517d89baecbc51a93f879 Mon Sep 17 00:00:00 2001 From: Eleriseth Date: Fri, 16 Nov 2012 23:00:18 +0000 Subject: [PATCH 1/6] PeerNode.shouldAcceptAnnounce never added uid to runningAnnounceUIDs BEWARE. TOTALLY UNTESTED CODE PATH ENABLED. This code never worked correctly since introduction in commit f32e365d2bd0b8529c641954cc62d4d86a2c089f (svn r16190). --- src/freenet/node/PeerNode.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index e8aefa11be9..85f4fe01c1c 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3899,6 +3899,7 @@ public synchronized boolean shouldAcceptAnnounce(long uid) { if(runningAnnounceUIDs.length > 0) System.arraycopy(runningAnnounceUIDs, 0, newList, 0, runningAnnounceUIDs.length); newList[runningAnnounceUIDs.length] = uid; + runningAnnounceUIDs = newList; timeLastAcceptedAnnouncement = now; return true; } else { From bc03e3e7b6e536906bcb7d645a5c1ac49bdb6c41 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Mon, 19 Nov 2012 12:14:17 +0000 Subject: [PATCH 2/6] Document what these methods are for. --- src/freenet/node/PeerNode.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 85f4fe01c1c..318c8449261 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3891,6 +3891,11 @@ protected void setJFKBuffer(byte[] bufferJFK) { private long timeLastAcceptedAnnouncement; private long[] runningAnnounceUIDs = new long[0]; + /** Protection against too many simultaneous announcements over a single + * connection. + * @param uid The announcement UID. + * @return True if we should accept the announcement. False to reject it. + */ public synchronized boolean shouldAcceptAnnounce(long uid) { long now = System.currentTimeMillis(); if(runningAnnounceUIDs.length < MAX_SIMULTANEOUS_ANNOUNCEMENTS && @@ -3907,6 +3912,7 @@ public synchronized boolean shouldAcceptAnnounce(long uid) { } } + /** Report that an announcement finished. */ public synchronized boolean completedAnnounce(long uid) { final int runningAnnounceUIDsLength = runningAnnounceUIDs.length; if(runningAnnounceUIDsLength < 1) return false; From abd9d7e9624e0c7dc09351ca2353621b562a8458 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Mon, 26 Nov 2012 12:32:07 +0000 Subject: [PATCH 3/6] Fix return value of completedAnnounce(), thanks Eleriseth. --- src/freenet/node/PeerNode.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 318c8449261..66bdd82f749 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3919,17 +3919,19 @@ public synchronized boolean completedAnnounce(long uid) { long[] newList = new long[runningAnnounceUIDsLength - 1]; int x = 0; for(int i=0;i Date: Mon, 26 Nov 2012 17:54:24 +0000 Subject: [PATCH 4/6] PeerNode.completedAnnounce was completely broken [ this is the actual bug fix, not the cleanup ] and we got off with that only because shouldAcceptAnnounce was broken too. --- src/freenet/node/PeerNode.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 66bdd82f749..7ff437ca97b 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3921,6 +3921,9 @@ public synchronized boolean completedAnnounce(long uid) { for(int i=0;i Date: Mon, 26 Nov 2012 17:56:58 +0000 Subject: [PATCH 5/6] Cleanup, thanks Eleriseth, this is the rest of "PeerNode.completedAnnounce was completely broken", not including the stuff about double announcement (which isn't relevant). --- src/freenet/node/PeerNode.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 7ff437ca97b..f38e7c4f821 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3914,12 +3914,10 @@ public synchronized boolean shouldAcceptAnnounce(long uid) { /** Report that an announcement finished. */ public synchronized boolean completedAnnounce(long uid) { - final int runningAnnounceUIDsLength = runningAnnounceUIDs.length; - if(runningAnnounceUIDsLength < 1) return false; - long[] newList = new long[runningAnnounceUIDsLength - 1]; + if(runningAnnounceUIDs.length < 1) return false; + long[] newList = new long[runningAnnounceUIDs.length - 1]; int x = 0; - for(int i=0;i Date: Tue, 1 Jan 2013 17:02:40 +0000 Subject: [PATCH 6/6] Fix unnecessary array copying. Add error messages for exceptional cases. Don't assert. Not the same as Eleriseth's patch, but thanks Eleriseth for pointing out the issue. --- src/freenet/node/PeerNode.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index f38e7c4f821..b6b828c58e9 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3919,18 +3919,21 @@ public synchronized boolean completedAnnounce(long uid) { int x = 0; for(long l : runningAnnounceUIDs) { if(l == uid) continue; - if(x == newList.length) + if(x == newList.length) { + Logger.warning(this, "UID not found in completedAnnounce, should not happen", new Exception("debug")); // uid was not found in runningAnnounceUIDs return false; + } newList[x++] = l; } - if(x < runningAnnounceUIDs.length) { + if(x < newList.length) { + Logger.error(this, "Duplicated UID, should not happen", new Exception("debug")); newList = Arrays.copyOf(newList, x); runningAnnounceUIDs = newList; return true; } else { - // not found - return false; + runningAnnounceUIDs = newList; + return true; } }