Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!weretis.net!feeder4.news.weretis.net!news.musoftware.de!wum.musoftware.de!fu-berlin.de!uni-berlin.de!individual.net!not-for-mail
From: Robert Klemme <shortcutter@googlemail.com>
Newsgroups: comp.lang.java.programmer
Subject: Re: Synchronization of the constructor
Date: Sat, 13 Aug 2011 12:17:33 +0200
Lines: 140
Message-ID: <9an1a5F3rrU1@mid.individual.net>
References: <j25hro$cjf$1@news.onet.pl>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
X-Trace: individual.net pUvg8O6O3s5uL6oeI1T3Kw3eBSFxplsfHgSdbOODe3Ye/tAqg=
Cancel-Lock: sha1:MdKB3XvzwSOyT8RcJ8CN8VBQS0c=
User-Agent: Mozilla/5.0 (Windows NT 6.0; WOW64; rv:5.0) Gecko/20110624 Thunderbird/5.0
In-Reply-To: <j25hro$cjf$1@news.onet.pl>
Xref: x330-a1.tempe.blueboxinc.net comp.lang.java.programmer:7073

On 13.08.2011 11:58, MaciekL wrote:

> I have a doubt because Java disables synchronization of the constructor
> by default.

Btw, it's "question" and not "doubt".  Side note: I see this quite 
often.  Does anybody have an idea why non native speakers often use 
"doubt" instead of "question"?  I'm not a native speaker myself but I 
can't remember having mixed up the two.  So I guess it has to do with 
the way English is taught or the mother tongue.

> Following example shows that lack of synchronization makes
> NullPointerException.
> ................
> [Thread[main,5,main]]  {TestApp(0)} Begin
> [Thread[runner,5,main]] Begin
> [Thread[runner,5,main]] Test
> Exception in thread "runner" java.lang.NullPointerException
>          at TestApp.test(TestApp.java:61)
>          at TestApp.run(TestApp.java:55)
>          at java.lang.Thread.run(Thread.java:662)
> [Thread[main,5,main]] Test
> [Thread[main,5,main]] OBJ = java.lang.Object@9304b1
> [Thread[main,5,main]]  {TestApp(0)} End
> ................

Where is line 61?  From what I can see your example only has about 30 lines.

> Synchronized 'test' method may be called by another thread in case
> the Object is not created.

What do you mean by that?

> This undefined behaviour is very tricky, note that following example
> may works correctly, it depends on the Constructor duration time.

You see an error.  That doesn't necessarily mean that behavior is undefined.

> I have found a soultion, adding whole definition of the Constructor into
> synchronized block.
> ...............
> synchronized (this)
> {
>    ... // ConstructorBody
> }
> ...............
>
> I don't understand why Java forbids simple declaration:
> ...............
> public synchronized TestApp(int timeout)
> ...............

Probably because usually "this" is not leaked (to other objects or 
threads) during construction.  This is generally considered dangerous 
because you leak a reference to a potentially incomplete instance (the 
constructor whose task it is to initialize the object to a consistent 
state did not yet finish).

I also remember that they changed the language spec at one point with 
regard to thread visibility of "this" during construction but I don't 
recall the details right now.

I have to say that your design is questionable: you inherit Runnable 
which is intended to decouple thread handling from what is executed in 
the thread.  Yet inside the constructor you create a thread and start it.

Rather you should first let the construction of the object finish 
properly and then create a Thread with that instance and start the 
thread.  If you want to do that in the same class a static method could 
do, e.g.

public static void runTest(final int timeout)
   throws InterruptedException {
   final TestApp ta = new TestApp();
   final Thread th = new Thread(ta);
   th.start();
   Thread.sleep(timeout);
   ta.setObj(new Object());
   ta.test();
   th.join();
}

> Because of that all calls to "add*Listener(this)" from constructor
> should be carrefully implemented.
>
> /*--:BEG:--[TestApp.java]----------------------------------------------------*/
> public class TestApp implements Runnable
> {
>    Object obj = null;
>    public static void info(String s)
>    {
>      System.err.println("[" + Thread.currentThread().toString() + "] " + s);
>    }
>    public TestApp(int timeout)
>    {
>      info(" {TestApp(" + timeout + ")} Begin");
>      Thread runner = (new Thread(this));
>      runner.setName("runner");
>      runner.start();
>      try { Thread.sleep(timeout); }
>      catch (InterruptedException ie) { }

Empty catch block without any handling or at least a comment explaining 
why you consciously ignore the exception is really bad.

>      obj = new Object();
>      test();
>      info(" {TestApp(" + timeout + ")} End");
>    }
>    public void run()
>    {
>      info("Begin");
>      test();
>      info("End");
>    }
>    public synchronized void test()
>    {
>      info("Test");
>      info("OBJ = " + obj.toString());

You don't need toString() - it's done automatically.

>    }
>    public static void main(String [] args)
>    {
>      new TestApp(1000);
>    }
> }
> /*--:EOF:--[TestApp.java]----------------------------------------------------*/

Now why do you want to start a thread in a constructor and have it 
executed code of "this"?

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/