On Tue, Jul 11, 2006 at 01:45:44PM +0200, Florent Daigni?re (NextGen$) wrote:
> * Matthew Toseland <toad at amphibian.dyndns.org> [2006-07-11 00:45:30]:
>
> > Is it necessary to take the lock while calling onFailure in the first
> > instance? And while calling start() in the second?
>
> We are writting lastInsertedPeers; we need the lock
>
> we don't need on start() indeed ... as we are properly synchronizing
> from inside : r9557 should fix it
Well, as you know, we should never take locks while doing blocking I/O,
unless it's just disk I/O and even then it's dodgy, and generally we
should minimize the time we are locked for.
>
> > On Sun, Jul 09, 2006 at 09:32:26PM +0000, nextgens at freenetproject.org
> > wrote:
> > > Author: nextgens
> > > Date: 2006-07-09 21:32:21 +0000 (Sun, 09 Jul 2006)
> > > New Revision: 9527
> > >
> > > Modified:
> > > trunk/freenet/src/freenet/client/async/BaseSingleFileFetcher.java
> > > trunk/freenet/src/freenet/node/Node.java
> > > Log:
> > > Consistent locking in MyARKInserter and BaseSingleFileFetcher
> > >
> > > Modified:
> > > trunk/freenet/src/freenet/client/async/BaseSingleFileFetcher.java
> > > ===================================================================
> > > --- trunk/freenet/src/freenet/client/async/BaseSingleFileFetcher.java
> > > 2006-07-09 21:21:37 UTC (rev 9526)
> > > +++ trunk/freenet/src/freenet/client/async/BaseSingleFileFetcher.java
> > > 2006-07-09 21:32:21 UTC (rev 9527)
> > > @@ -12,7 +12,7 @@
> > > public abstract class BaseSingleFileFetcher implements SendableGet {
> > >
> > > final ClientKey key;
> > > - protected boolean cancelled;
> > > + private boolean cancelled;
> > > final int maxRetries;
> > > private int retryCount;
> > > final FetcherContext ctx;
> > > @@ -32,9 +32,11 @@
> > >
> > > /** Do the request, blocking. Called by RequestStarter. */
> > > public void send(Node node) {
> > > - if(cancelled) {
> > > - onFailure(new
> > > LowLevelGetException(LowLevelGetException.CANCELLED));
> > > - return;
> > > + synchronized (this) {
> > > + if(cancelled) {
> > > + onFailure(new
> > > LowLevelGetException(LowLevelGetException.CANCELLED));
> > > + return;
> > > + }
> > > }
> > > // Do we need to support the last 3?
> > > ClientKeyBlock block;
> > >
> > > Modified: trunk/freenet/src/freenet/node/Node.java
> > > ===================================================================
> > > --- trunk/freenet/src/freenet/node/Node.java 2006-07-09 21:21:37 UTC
> > > (rev 9526)
> > > +++ trunk/freenet/src/freenet/node/Node.java 2006-07-09 21:32:21 UTC
> > > (rev 9527)
> > > @@ -189,11 +189,13 @@
> > > return true; // no point inserting
> > > }
> > > Peer[] p = getPrimaryIPAddress();
> > > - if(lastInsertedPeers != null) {
> > > - if(p.length != lastInsertedPeers.length) return
> > > true;
> > > - for(int i=0;i<p.length;i++)
> > > -
> > > if(!p[i].strictEquals(lastInsertedPeers[i]))
> > > - return true;
> > > + synchronized (this) {
> > > + if(lastInsertedPeers != null) {
> > > + if(p.length !=
> > > lastInsertedPeers.length) return true;
> > > + for(int i=0;i<p.length;i++)
> > > +
> > > if(!p[i].strictEquals(lastInsertedPeers[i]))
> > > + return true;
> > > + }
> > > }
> > > return false;
> > > }
> > > @@ -226,30 +228,32 @@
> > >
> > > Logger.minor(this, "Inserting ARK: "+uri);
> > >
> > > - inserter = new ClientPutter(this, b, uri,
> > > - new ClientMetadata("text/plain") /* it
> > > won't quite fit in an SSK anyway */,
> > > -
> > > Node.this.makeClient((short)0).getInserterContext(),
> > > - chkPutScheduler, sskPutScheduler,
> > > RequestStarter.INTERACTIVE_PRIORITY_CLASS, false, false, this);
> > > -
> > > - try {
> > > - inserter.start();
> > > - if(fs.get("physical.udp") == null)
> > > - lastInsertedPeers = null;
> > > - else {
> > > - try {
> > > - String[] all =
> > > fs.getAll("physical.udp");
> > > - Peer[] peers = new
> > > Peer[all.length];
> > > - for(int i=0;i<all.length;i++)
> > > - peers[i] = new
> > > Peer(all[i], false);
> > > - lastInsertedPeers = peers;
> > > - } catch (PeerParseException e1) {
> > > - Logger.error(this, "Error
> > > parsing own ref: "+e1+" : "+fs.get("physical.udp"), e1);
> > > - } catch (UnknownHostException e1) {
> > > - Logger.error(this, "Error
> > > parsing own ref: "+e1+" : "+fs.get("physical.udp"), e1);
> > > + synchronized (this) {
> > > + inserter = new ClientPutter(this, b, uri,
> > > + new
> > > ClientMetadata("text/plain") /* it won't quite fit in an SSK anyway */,
> > > +
> > > Node.this.makeClient((short)0).getInserterContext(),
> > > + chkPutScheduler,
> > > sskPutScheduler, RequestStarter.INTERACTIVE_PRIORITY_CLASS, false, false,
> > > this);
> > > +
> > > + try {
> > > + inserter.start();
> > > + if(fs.get("physical.udp") == null)
> > > + lastInsertedPeers = null;
> > > + else {
> > > + try {
> > > + String[] all =
> > > fs.getAll("physical.udp");
> > > + Peer[] peers = new
> > > Peer[all.length];
> > > + for(int
> > > i=0;i<all.length;i++)
> > > + peers[i] = new
> > > Peer(all[i], false);
> > > + lastInsertedPeers =
> > > peers;
> > > + } catch (PeerParseException e1)
> > > {
> > > + Logger.error(this,
> > > "Error parsing own ref: "+e1+" : "+fs.get("physical.udp"), e1);
> > > + } catch (UnknownHostException
> > > e1) {
> > > + Logger.error(this,
> > > "Error parsing own ref: "+e1+" : "+fs.get("physical.udp"), e1);
> > > + }
> > > }
> > > + } catch (InserterException e) {
> > > + onFailure(e, inserter);
> > > }
> > > - } catch (InserterException e) {
> > > - onFailure(e, inserter);
> > > }
> > > }
> > >
> > >
> > > _______________________________________________
> > > cvs mailing list
> > > cvs at freenetproject.org
> > > http://emu.freenetproject.org/cgi-bin/mailman/listinfo/cvs
> > >
> >
> > --
> > Matthew J Toseland - toad at amphibian.dyndns.org
> > Freenet Project Official Codemonkey - http://freenetproject.org/
> > ICTHUS - Nothing is impossible. Our Boss says so.
>
>
--
Matthew J Toseland - toad at amphibian.dyndns.org
Freenet Project Official Codemonkey - http://freenetproject.org/
ICTHUS - Nothing is impossible. Our Boss says so.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<https://emu.freenetproject.org/pipermail/cvs/attachments/20060711/56c87cac/attachment.pgp>
