On Apr 5, 2008, at 8:18 AM, toad at freenetproject.org wrote:
> Author: toad
> Date: 2008-04-05 13:18:53 +0000 (Sat, 05 Apr 2008)
> New Revision: 19014
>
> Modified:
> trunk/freenet/src/freenet/node/SSKInsertSender.java
> Log:
> Synchronize on access to status.
> Inspired by Daniel Cheng's patch.
>
> Modified: trunk/freenet/src/freenet/node/SSKInsertSender.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/SSKInsertSender.java 2008-04-05
> 13:16:17 UTC (rev 19013)
> +++ trunk/freenet/src/freenet/node/SSKInsertSender.java 2008-04-05
> 13:18:53 UTC (rev 19014)
> @@ -525,7 +525,7 @@
> // Nothing to wait for, no downstream transfers, just exit.
> }
>
> - public int getStatus() {
> + public synchronized int getStatus() {
> return status;
> }
>
> @@ -536,7 +536,7 @@
> /**
> * @return The current status as a string
> */
> - public String getStatusString() {
> + public synchronized String getStatusString() {
> if(status == SUCCESS)
> return "SUCCESS";
> if(status == ROUTE_NOT_FOUND)
>
I have not really examined nextgens recent volatile patches, but my
understanding is that this is actually one of the limited cases where
a volatile field is exactly what is desired. Other threads (like
fproxy, logging) would have to block to read the status, but the
synchronization does not help since the moment the status is returned
any atomicity is lost. The return value of getStatus()/
getStatusString() is therefore inherently 'dirty'.
Consider the following patch.
--
Robert Hailey
Index: src/freenet/node/SSKInsertSender.java
===================================================================
--- src/freenet/node/SSKInsertSender.java (revision 19061)
+++ src/freenet/node/SSKInsertSender.java (working copy)
@@ -59,7 +59,7 @@
private SSKBlock block;
private static boolean logMINOR;
- private int status = -1;
+ private volatile int status = -1;
/** Still running */
static final int NOT_FINISHED = -1;
/** Successful insert */
@@ -525,7 +525,7 @@
// Nothing to wait for, no downstream transfers, just exit.
}
- public synchronized int getStatus() {
+ public int getStatus() {
return status;
}
@@ -536,7 +536,10 @@
/**
* @return The current status as a string
*/
- public synchronized String getStatusString() {
+ public String getStatusString() {
+ //status is volatile, get a value only once.
+ int status=this.status;
+
if(status == SUCCESS)
return "SUCCESS";
if(status == ROUTE_NOT_FOUND)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080407/76e2983c/attachment.html>
