<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><html>On Apr 5, 2008, at 8:18 AM, <a href="mailto:toad@freenetproject.org">toad@freenetproject.org</a> wrote:</html><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: toad<br>Date: 2008-04-05 13:18:53 +0000 (Sat, 05 Apr 2008)<br>New Revision: 19014<br><br>Modified:<br> trunk/freenet/src/freenet/node/SSKInsertSender.java<br>Log:<br>Synchronize on access to status.<br>Inspired by Daniel Cheng's patch.<br><br>Modified: trunk/freenet/src/freenet/node/SSKInsertSender.java<br>===================================================================<br>--- trunk/freenet/src/freenet/node/SSKInsertSender.java<span class="Apple-tab-span" style="white-space:pre"> </span>2008-04-05 13:16:17 UTC (rev 19013)<br>+++ trunk/freenet/src/freenet/node/SSKInsertSender.java<span class="Apple-tab-span" style="white-space:pre"> </span>2008-04-05 13:18:53 UTC (rev 19014)<br>@@ -525,7 +525,7 @@<br> // Nothing to wait for, no downstream transfers, just exit.<br> }<br><br>- public int getStatus() {<br>+ public synchronized int getStatus() {<br> return status;<br> }<br><br>@@ -536,7 +536,7 @@<br> /**<br> * @return The current status as a string<br> */<br>- public String getStatusString() {<br>+ public synchronized String getStatusString() {<br> if(status == SUCCESS)<br> return "SUCCESS";<br> if(status == ROUTE_NOT_FOUND)<br><br></div></blockquote><div><br></div></div>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'.<div><br></div><div>Consider the following patch.</div><div><br></div><div>--</div><div>Robert Hailey</div><div><br></div><div><div><font class="Apple-style-span" face="Courier">Index: src/freenet/node/SSKInsertSender.java</font></div><div><font class="Apple-style-span" face="Courier">===================================================================</font></div><div><font class="Apple-style-span" face="Courier">--- src/freenet/node/SSKInsertSender.java</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" face="Courier"> </font></span><font class="Apple-style-span" face="Courier">(revision 19061)</font></div><div><font class="Apple-style-span" face="Courier">+++ src/freenet/node/SSKInsertSender.java</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" face="Courier"> </font></span><font class="Apple-style-span" face="Courier">(working copy)</font></div><div><font class="Apple-style-span" face="Courier">@@ -59,7 +59,7 @@</font></div><div><font class="Apple-style-span" face="Courier"> private SSKBlock block;</font></div><div><font class="Apple-style-span" face="Courier"> private static boolean logMINOR;</font></div><div><font class="Apple-style-span" face="Courier"> </font></div><div><font class="Apple-style-span" face="Courier">- private int status = -1;</font></div><div><font class="Apple-style-span" face="Courier">+ private volatile int status = -1;</font></div><div><font class="Apple-style-span" face="Courier"> /** Still running */</font></div><div><font class="Apple-style-span" face="Courier"> static final int NOT_FINISHED = -1;</font></div><div><font class="Apple-style-span" face="Courier"> /** Successful insert */</font></div><div><font class="Apple-style-span" face="Courier">@@ -525,7 +525,7 @@</font></div><div><font class="Apple-style-span" face="Courier"> // Nothing to wait for, no downstream transfers, just exit.</font></div><div><font class="Apple-style-span" face="Courier"> }</font></div><div><font class="Apple-style-span" face="Courier"> </font></div><div><font class="Apple-style-span" face="Courier">- public synchronized int getStatus() {</font></div><div><font class="Apple-style-span" face="Courier">+ public int getStatus() {</font></div><div><font class="Apple-style-span" face="Courier"> return status;</font></div><div><font class="Apple-style-span" face="Courier"> }</font></div><div><font class="Apple-style-span" face="Courier"> </font></div><div><font class="Apple-style-span" face="Courier">@@ -536,7 +536,10 @@</font></div><div><font class="Apple-style-span" face="Courier"> /**</font></div><div><font class="Apple-style-span" face="Courier"> * @return The current status as a string</font></div><div><font class="Apple-style-span" face="Courier"> */</font></div><div><font class="Apple-style-span" face="Courier">- public synchronized String getStatusString() {</font></div><div><font class="Apple-style-span" face="Courier">+ public String getStatusString() {</font></div><div><font class="Apple-style-span" face="Courier">+</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" face="Courier"> </font></span><font class="Apple-style-span" face="Courier">//status is volatile, get a value only once.</font></div><div><font class="Apple-style-span" face="Courier">+</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" face="Courier"> </font></span><font class="Apple-style-span" face="Courier">int status=this.status;</font></div><div><font class="Apple-style-span" face="Courier">+</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" face="Courier"> </font></span></div><div><font class="Apple-style-span" face="Courier"> if(status == SUCCESS)</font></div><div><font class="Apple-style-span" face="Courier"> return "SUCCESS";</font></div><div><font class="Apple-style-span" face="Courier"> if(status == ROUTE_NOT_FOUND)</font></div><div><br></div></div></body></html>