You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Sid Stuart <si...@weaselworks.com> on 2003/04/24 16:30:26 UTC
Possible bug in org.apache.james.James.getId()
Hi,
This version of the James class came from the CVS repository. In it, the
method getID() looks like,
public String getId() {
long localCount = -1;
synchronized (TinyJames.class) {
localCount = count++;
}
StringBuffer idBuffer =
new StringBuffer(64)
.append("Mail")
.append(System.currentTimeMillis())
.append("-")
.append(count++);
return idBuffer.toString();
}
In the third to last line,
.append(count++)
the global variable count is being incremented outside of a syncronized
block. Since this is supposed to be thread-safe code, that is a bad
thing. The line should look like,
.append(localCount)
Sid
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
Re: Possible bug in org.apache.james.James.getId()
Posted by Sid Stuart <si...@weaselworks.com>.
Noel J. Bergman wrote:
>Sid,
>
>I agree with the basic claim, but the code you posted does not match the
>content of the CVS. Specifically, "TinyJames" is not in the CVS.
>
>It appears that Peter made the change to use the synchronized block last
>August, but didn't catch the fact that the local variable was never used.
>That also results in the message ID skipping every other value.
>
> --- Noel
>
>
Sorry, I was playing with a subset (TinyJames) of James when I noticed
the bug. Copied it from the wrong class file and didn't notice that Idea
had refactored the class name in the Syncronized block. You really had
me going for a minute when you said it came from TinyJames. :)
Sid
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
RE: Possible bug in org.apache.james.James.getId()
Posted by "Noel J. Bergman" <no...@devtech.com>.
Sid,
I agree with the basic claim, but the code you posted does not match the
content of the CVS. Specifically, "TinyJames" is not in the CVS.
It appears that Peter made the change to use the synchronized block last
August, but didn't catch the fact that the local variable was never used.
That also results in the message ID skipping every other value.
--- Noel
-----Original Message-----
From: Sid Stuart [mailto:sid@weaselworks.com]
Sent: Thursday, April 24, 2003 10:30
To: James Developers List
Subject: Possible bug in org.apache.james.James.getId()
Hi,
This version of the James class came from the CVS repository. In it, the
method getID() looks like,
public String getId() {
long localCount = -1;
synchronized (TinyJames.class) {
localCount = count++;
}
StringBuffer idBuffer =
new StringBuffer(64)
.append("Mail")
.append(System.currentTimeMillis())
.append("-")
.append(count++);
return idBuffer.toString();
}
In the third to last line,
.append(count++)
the global variable count is being incremented outside of a syncronized
block. Since this is supposed to be thread-safe code, that is a bad
thing. The line should look like,
.append(localCount)
Sid
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org