On Thu, 15 Jun 2006 01:37:35 +0100, Matthew Toseland wrote:

> On Thu, Jun 15, 2006 at 12:45:02AM +0300, Jusa Saari wrote:

>> > Also the point about starting threads in constructors in non-final
>> > classes may impact us. It is also possible that some code violates
>> > this:
>> 
>> Uh oh. I've seen code that started threads from constructors (in
>> FreeCol) break (begin throwing NullPointerExceptions) just by changing
>> the garbage collector, and begin functioning properly when I fixed the
>> code (moved the thread starting into another method and changed all
>> places that created objects into looking like "new
>> SomeObject().startThreads()").
>> 
>> Anyway, if there's such code in Freenet, it *will* cause problems.
> 
> There is, though we always start the thread at the end of the constructor,
> and as I understand that page Thread.start() will flush everything in the
> surrounding code first...

That makes no difference - the code I saw also started the thread as the
last thing in the constructor. From what I've understood the problem is
that the "final" fields are not frozen until the constructor actually
finishes, the compiler and JVM are free to rearrange the code inside the
constructor for maximum efficiency (so the thread may actually be started
before you think it would just from reading the source), and even if they
didn't any subclass is going to call the superclass constructor as the
first thing in its own constructor.

Anyway, no matter what the theoretical roots of the problem, I've seen it
cause weird problems in other code, so it needs to be fixed.

> File a bug, or hack on it yourself.

I'll look into it - I have some spare time, but I'm not very familiar with
Freenet's codebase.

Anyway, there's at least two good models to fix this problem: the init()
way and the factory way.

The init() way simply includes additional method, init(), which starts the
thread (and adds reference to the object to where ever it needs to be
added - the construct *must not* leak references), so all object creations
change from "new Object()" into "new Object().init()". It has the downside
of leading to bugs if the programmer forgots to call init(), but it works
well even if the class is subclassed, since the init() method is inherited.

The factory way is the good old trick of making the constructor
protected, and providing a static method for getting new objects. That
static method constructs the object, calls its (also protected) init
method, and returns it. It has the downside of making subclassing a bit
more complex - you need to override the factory method - but protects
against anyone forgetting to call init() (or multiple calls to it, for
that matter).

Which way would be preferable to use in Freenet ? I'd recommend factory,
since there's less chances of anything going wrong since the programmer
doesn't need to remember to call init(), but you're the professional
programmer here :).

>> > "The model also allows inconsistent visibility in the absence of
>> > synchronization. For example, it is possible to obtain a fresh value
>> > for one field of an object, but a stale value for another. Similarly,
>> > it is possible to read a fresh, updated value of a reference variable,
>> > but a stale value of one of the fields of the object now being
>> > referenced."
>> > 
>> > On Wed, Jun 14, 2006 at 10:16:47PM +0300, Jusa Saari wrote:
>> >> On Wed, 14 Jun 2006 13:57:38 +0100, Matthew Toseland wrote:
>> >> 
>> >> > On Wed, Jun 14, 2006 at 12:59:56PM +0300, Jusa Saari wrote:
>> >> >> On Tue, 13 Jun 2006 20:34:05 +0100, Matthew Toseland wrote:
>> >> >> 
>> >> >> > On Tue, Jun 13, 2006 at 10:26:55PM +0300, Jusa Saari wrote:
>> >> >> >> What happens if the watchdog gets stuck too ? It has to
>> >> >> >> synchronize with the watched thread sometimes to do its work,
>> >> >> >> AFAIK.
>> >> >> > 
>> >> >> > It just reads a variable. An int. Without synchronization.
>> >> >> 
>> >> >> I hope that's a "volatile" variable ?
>> >> > 
>> >> > Is that necessary?
>> >> 
>> >> Yes. It guarantees that the watchdog thread sees any updates made by
>> >> the watched thread, instead of seeing a possibly stale value in local
>> >> CPU cache. Not using "volatile" keyword in variable declaration is
>> >> not an issue in an X86 platform, since the X86 architechture gives
>> >> stricter cache coherency guarantees than Java Memory Model, but it's
>> >> going to lead to really nasty bugs in any architechture that doesn't
>> >> give them.
>> >> 
>> >> Basically "volatile" guarantees that each thread will see the changes
>> >> made by any other thread, without needing to synchronize. It also
>> >> guarantees that longs and other larger-than-32-bit variables will be
>> >> read and written atomically, but that doesn't matter for ints,
>> >> obviously.
>> >> 
>> >> Here's a bit more info - see the bottom of the page:
>> >> 
>> >> http://g.oswego.edu/dl/cpj/jmm.html
>> >> 
>> >> _______________________________________________ Devl mailing list
>> >> Devl@freenetproject.org
>> >> http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
>> >>
>> >>
>> 
>> _______________________________________________ Devl mailing list
>> Devl@freenetproject.org
>> http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
>>


_______________________________________________
Devl mailing list
Devl@freenetproject.org
http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to