Synchronization of the constructor

[MaciekL]

Hi,

I have a doubt because Java disables synchronization of the constructor
by default.
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
.................

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

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

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)
................

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

Regards

 

[Robert Klemme]

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

 

[Arved Sandstrom]

"Robert Klemme" wrote in message

It's not confusion. In some cultures, especially those from the
subcontinent, "doubt" actually means something slightly different from
Western cultures. In those cultures it is used when something is not
completely understood and embodies the question that results from that
incomplete understanding.

That would actually be a British or North American English meaning too.
"You're doubtful because you're uncertain" kind of thing. Or simply that
"you have doubts because you are uncertain". After all, in our wonderful
world of IT, you may say that you are doubtful not because you are
skeptical but only because something is unknown or unresolved.

Thing is that in Western English we tend to use "doubt", as a verb or
noun, in a restrictive sense that implies skepticism or a tendency
towards disbelief. So it becomes difficult to use the word to suggest
_uncertainty_, which is obviously still a valid use, and is what I think
all these other speakers mean. In fact I think their usage is becoming
an archaism in Western English. But it doesn't make their usage wrong.

Part of the problem too stems from a difference between standard and
Indian English as to the countability of the noun "doubt". In standard
English we have singular "doubt" and plural "doubts", but that doesn't
mean you can say you have two doubts or _a_ doubt. Apparently in Indian
English you can. They'd preserve the sense of what they are trying to
say, and immediately make themselves better understood to Western
English speakers, if they said "I have doubts...". With the caveat that
usually we garner a sense of disbelief or skepticism from that, where
none exists.

AHS

 

[Eric Sosman]

Hi,

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

The constructor can be synchronized, but not on the object
being constructed. The `this' object is not a full-fledged
instance until the constructor finishes -- more generally, until
all its constructors finish, including those of subclasses.

While the instance is in an incomplete and fragile state,
before it's completely constructed, you must be careful not to
let it be "seen" outside its chain of constructors. Java forbids
the bare `synchronized' on a constructor because the object you
want to synchronize on doesn't truly exist yet. If some other
thread can "see" the unformed object, that's an error in your
code -- and if no other thread can "see" it, it needs no
synchronization anyhow.

[...]
I have found a soultion, adding whole definition of the Constructor into
synchronized block.
...............
synchronized (this)
{
... // ConstructorBody
}

That's not a solution; it just hides your error so the Java
compiler doesn't notice it. The error message disappears, but the
error itself remains.

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));

... and here's the error. The `this' object is still incomplete,
not fully able to carry out its duties as a TestApp instance. Yet you
have given it to the new Thread to play with, and the Thread expects
`this' to be able to function as a full-fledged TestApp. The `this'
object is still in the womb, but the Thread needs it to behave as a
full-grown adult.

Don't Do That. Always let the constructor(s) run to completion
before revealing the new instance to the rest of the program. Don't
give a `this' reference to another thread, or put it in a globally-
accessible array or collection, or even call a class method that may
have been overridden in a subclass. Until they reach maturity, you
must not ask children to assume adult responsibilities.

 

[markspace]

The constructor can be synchronized,

It really can't. Recall that the compiler will insert a call to a super
constructor if the first statement doesn't have a such a call or a call
to another class constructor. Consider this:

public class SomeClass {
public SomeClass() {
synchronized( SomeClass.class ) {
...
}
}
....

What you get is this:

public class SomeClass {
public SomeClass() {
super();
synchronized( SomeClass.class ) {
...
}
}
....

And thus you see that some writes in the construction occur outside of
the synchronized block, a classic case of incorrectly written
synchronization.

The closest Java gets to a synchronized constructor is immutable objects
made with final fields.

public class Immutable {
private final SomeObject o;
public Immutable() {
o = new SomeObject();
}
....

This is thread safe, and immutable, because the fields written are
declared "final." Java takes special processing at the end of the
constructor to synchronize all final fields, and any writes made to
objects accessible via those final fields, with all other threads in the
system. So now this Immutable class can be used safely by any thread in
the system.

 

[Lew]

It really can't. Recall that the compiler will insert a call to a super
constructor if the first statement doesn't have a such a call or a call
to another class constructor. Consider this:

public class SomeClass {
public SomeClass() {
synchronized( SomeClass.class ) {
...
}
}
...

What you get is this:

public class SomeClass {
public SomeClass() {
super();
synchronized( SomeClass.class ) {
...
}
}
...

And thus you see that some writes in the construction occur outside of
the synchronized block, a classic case of incorrectly written
synchronization.

The closest Java gets to a synchronized constructor is immutable objects
made with final fields.

public class Immutable {
private final SomeObject o;
public Immutable() {
o = new SomeObject();
}
...

This is thread safe, and immutable, because the fields written are
declared "final." Java takes special processing at the end of the
constructor to synchronize all final fields, and any writes made to
objects accessible via those final fields, with all other threads in the
system. So now this Immutable class can be used safely by any thread in
the system.

Others have explained your error quite well, OP, so I will merely add that you should read the Java Language Specification on threads, synchronizationand the /happens-before/ relationship. Also, read and study /Effective Java/ by Joshua Bloch, /Java Concurrency in Practice/ by Brian Goetz, et al.,the book by Doug Lea whose title escapes me just now (lmgtfy?) and any article in the IBM Developerworks for Java site's series, especially those by Goetz, Bloch and others on the topic of threaded Java code.

Bottom line: constructors are wholly for _construction_, only constructionand nothing but construction, not operations on the object. Do your logicoutside the constructor, after the constructor's promises have a chance tobe kept.

 

[Lew]

I have been shifting away from public constructors towards private or
protected constructors with a public static factory method.

In this approach, it is easy to make the constructor do absolutely
nothing but construction. The factory method can do additional steps,
such as starting a thread or calling synchronized methods, to bring the
object into full operation before returning it.

In addition to making it easier to write thread-safe code, the factory
method approach is generally more flexible than calling constructors
directly from other classes.

In addition to the benefits Patricia mentioned, factory methods can infer generic parameters in a lot of situations where you'd have to state them explicitly with constructors. They can also call builders for you, if you have that need.

I do recommend that one not make a religion out of factory methods. Badly designed factory methods are especially evil. (I remember one project where the "factory" method required explicit knowledge of the class under construction, had naming conventions connecting the method to both the class under construction and the associated unit test that had to be followed or thecode broke, abandoned compiler-enforceable type relationships, and required a cast to the target type anyway, all to accomplish a simple no-argument constructor call of the target type.) Then again, badly-designed code is always evil, isn't it?

Point being that there are particular use cases when it's OK to use a constructor if the advantages of a factory method are not particularly compelling, or if the hoops to implement and use a factory method are too epicyclic.But Patricia's advice and insights are very sound (as they always are from her); factory methods bring great advantages for a lot of scenarios. I certainly plan to increase my appreciation for them because of her post.

 

[kedar mhaswade]

It really can't.  Recall that the compiler will insert a call to a super
constructor if the first statement doesn't have a such a call or a call
to another class constructor.  Consider this:

public class SomeClass {
   public SomeClass() {
     synchronized( SomeClass.class ) {
       ...
     }
   }
...

What you get is this:

public class SomeClass {
   public SomeClass() {
     super();
     synchronized( SomeClass.class ) {
       ...
     }
   }
...

And thus you see that some writes in the construction occur outside of
the synchronized block, a classic case of incorrectly written
synchronization.

The closest Java gets to a synchronized constructor is immutable objects
made with final fields.

public class Immutable {
   private final SomeObject o;
   public Immutable() {
     o = new SomeObject();
   }
...

This is thread safe, and immutable, because the fields written are
declared "final."  Java takes special processing at the end of the
constructor to synchronize all final fields, and any writes made to
objects accessible via those final fields, with all other threads in the
system.  So now this Immutable class can be used safely by any thread in
the system.

Doesn't that depend on how Immutable SomeObject is and how Immutable's
immutability relates to it?
Also, other standard caveats apply (e.g. Immutable should be declared
final).

 

[kedar mhaswade]

In addition to the benefits Patricia mentioned, factory methods can infergeneric parameters in a lot of situations where you'd have to state them explicitly with constructors.  They can also call builders for you, if youhave that need.

I do recommend that one not make a religion out of factory methods.  Badly designed factory methods are especially evil.  (I remember one project where the "factory" method required explicit knowledge of the class underconstruction, had naming conventions connecting the method to both the class under construction and the associated unit test that had to be followed or the code broke, abandoned compiler-enforceable type relationships, and required a cast to the target type anyway, all to accomplish a simple no-argument constructor call of the target type.)  Then again, badly-designed code is always evil, isn't it?

Point being that there are particular use cases when it's OK to use a constructor if the advantages of a factory method are not particularly compelling, or if the hoops to implement and use a factory method are too epicyclic.  But Patricia's advice and insights are very sound (as they always arefrom her); factory methods bring great advantages for a lot of scenarios.  I certainly plan to increase my appreciation for them because of her post.

I think this is also the advice that EJ-II edition-Item 1 give us --
Favor static factory methods. However, nowadays, there's yet another
popular (and effective) way of constructing objects and that's
delegating it to a DI library like Guice via injection.

 

[markspace]

Doesn't that depend on how Immutable SomeObject is and how Immutable's
immutability relates to it?

No! That's the point. Even something like this:

public class Stooges {

private final ArrayList stooges;

public Stooges() {
ArrayList temp = new ArrayList(3);
temp.add( "Larry" );
temp.add( "Moe" );
temp.add( "Curly" );
stooges = temp;
}
public List getStooges() {
return new ArrayList( stooges );
}
}

Where "SomeObject" is known to be mutable (here, it's an ArrayList), is
still thread safe and immutable. Thread safety and immutability here is
just the absence of writes. That's all we have to do to to be safe.
And Java guarantees that the inital writes in the constructor are
"flushed" before any other thread can see them. Thus this object is
thread safe.

Note that I:

1. Don't modify any memory after I create the immutable object. I.e.,
"stooges" is never modified.

2. I don't allow the user to do so either.

3. I don't allow a reference to "stooges" to escape. If one were passed
in, I'd be obliged to copy the whole thing. Otherwise, someone else
could be holding on to the reference and modify the object behind my
back, thus breaking immutability and thread safety.

Also, other standard caveats apply (e.g. Immutable should be declared
final).

Nope. Certainly if you extend a class and then break its published
constraints, you've done something bad. But that applies to every class
in the system. The only caveat I'd apply is to document the
immutability of the class. Once you've done that, you've done all you
can do, really.

In a public API, sure, declaring the class final is worth considering,
but programmatically it's not required.

Publishing the reference you get still has to be considered, but that is
also for the user of this class to address. We simply can't do anything
about it.

 

[markspace]

public class Stooges {

private final ArrayList stooges;

public Stooges() {
ArrayList temp = new ArrayList(3);
temp.add( "Larry" );
temp.add( "Moe" );
temp.add( "Curly" );
stooges = temp;
}
public List getStooges() {
return new ArrayList( stooges );
}
}

I want to make one small change here. The above is correct, but
misleading. Both of the following are also immutable and thread safe:


public class Stooges {

private final ArrayList stooges;

public Stooges() {
ArrayList stooges = new ArrayList(3);
stooges.add( "Larry" );
stooges.add( "Moe" );
stooges.add( "Curly" );
}
public List getStooges() {
return new ArrayList( stooges );
}
}


-- or --

public class Stooges {

private final ArrayList stooges = new ArrayList(3);

public Stooges() {
stooges.add( "Larry" );
stooges.add( "Moe" );
stooges.add( "Curly" );
}
public List getStooges() {
return new ArrayList( stooges );
}
}

 

[Lew]

I want to make one small change here. The above is correct, but
misleading. Both of the following are also immutable and thread safe:


public class Stooges {

private final ArrayList stooges;

public Stooges() {
ArrayList stooges = new ArrayList(3);
stooges.add( "Larry" );
stooges.add( "Moe" );
stooges.add( "Curly" );
}
public List getStooges() {
return new ArrayList( stooges );
}
}


-- or --

public class Stooges {

private final ArrayList stooges = new ArrayList(3);

public Stooges() {
stooges.add( "Larry" );
stooges.add( "Moe" );
stooges.add( "Curly" );
}
public List getStooges() {
return new ArrayList( stooges );
}
}

See JLS §17.5:
"The usage model for final fields is a simple one. Set the final fields foran object in that object's constructor. Do not write a reference to the object being constructed in a place where another thread can see it before the object's constructor is finished. If this is followed, then when the object is seen by another thread, that thread will always see the correctly constructed version of that object's final fields. It will also see versions of any object or array referenced by those final fields that are at least asup-to-date as the final fields are."

 

[markspace]

See JLS §17.5: "...It will also see versions of
any object or array referenced by those final fields that are at
least as up-to-date as the final fields are."

This is a little misleading, I think. In fact the update does not occur
when the final field is written ("at least as up-to-date as the final
fields are"). The update occurs at the end of the constructor.

JLS 17.5.1:
"The semantics for final fields are as follows. Let o be an object, and
c be a constructor for o in which f is written. A freeze action on a
final field f of o takes place when c exits, either normally or abruptly."

The "freeze action" is a little unclear. It's not technically a
synchronization, because immutable objects are specifically stated to
not use synchronization. It is a happens-before though (even though
that language isn't used), and it appears to imply a memory-barrier.

I'm not sure exactly why they use "freeze action" and not
happens-before. There might be some subtle difference for compiler
writers, than doesn't affect the rest of us.

 

[Lew]

Lew wrote:
[snip]

Point being that there are particular use cases when it's OK to use a
constructor if the advantages of a factory method are not particularly
compelling, or if the hoops to implement and use a factory method are
too epicyclic. But Patricia's advice and insights are very sound (as they
always are from her); factory methods bring great advantages for a lot of
scenarios. I certainly plan to increase my appreciation for them
because of her post.

In general is it better to have static factory methods on the class that is
to be instantiated or to have a separate factory class?

That is a very good question.

I don't think there is a general rule here. You have to do what is smart for the given situation. Either way, make sure the factory method is well written.

If the construction is tightly bound to the class under construction, that would argue for the static method belonging to the class to be built. OTOH, if the factory is one of a class of factories, e.g., for collections, then it might make more sense to push it out into a helper class. Do what makes for the cleanest design that is easiest to maintain over the long term for the scenario. One goal is to make it easy to figure out where the factory is when you need instances. Another is to have compiler-enforced type safety.

Also, there may be cases where the factory method should be an instance method of a separate class, e.g., if you want to load a configuration to guidethe construction and have different instances work from different configurations.

Do what makes sense.