On Apr 5, 2008, at 8:18 AM, [EMAIL PROTECTED] 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)

_______________________________________________
Devl mailing list
[email protected]
http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to