[concurrency-interest] Starting a Thread within a constructor, what can go wrong?

Tim Halloran hallorant at gmail.com
Fri Dec 10 22:12:05 EST 2010


On Fri, Dec 10, 2010 at 9:51 PM, Marco Villalobos <mvillalobos at kineteque.com
> wrote:

> I see.  David, thank you for the quick reply.
>
> Goetz said in his excellent book, Concurrency in Practice that,
> "publishing objects before they are fully constructed can compromise
> thread safety."  That "publishing an object from within its
> constructor can publish an incompletely constructed object.  This is
> true even if the publication is the last statement in the
> constructor."
>
> This language really makes me believe that such an example would break
> concurrency.
>
> However, you're stating that if a programmer pays careful attention to
> the "happens-before" semantics, then thread safety could be insured
> (even though it is not a good practice).
>

This is true.


>
> Earlier today, posted the same question to Brian Goetz.  He replied
> stating:
>
> QUOTE
>
> "The biggest risk is that allowing the 'this' object to escape during
> construction voids all the final field visibility guarantees, which
> means that immutable objects can be observed to changed their value
> after construction, objects can be observed to not respect invariants
> set up in constructors, etc.)
>
> Starting a thread from within a constructor can sometimes be harmless,
> but most use cases that call for starting a thread from the
> constructor also call for the run() method of that thread to access
> the state of the object being constructed, and in that case all bets
> are off as to the validity of the state it sees.  So starting a thread
> from a constructor is a prime vector through which 'this' can escape
> construction and all sorts of weird things can ensue."
>
> END QUOTE
>
> I'll try writing test cases to prove that even happens-before
> semantics are broken in this situation.  I'll pay attention to the
> immutable fields, and even add a thread sleep to see if that show this
> practice indeed does break thread-safety.
>
> Hopefully my exercise is not a useless one.
>

Testing may not be fruitful in this situation as some VM implementations are
conservative in their implementation of the Java memory model and they mask
some "bad things" from happening. Change the hardware or the VM version and
intermittent failures may begin. As you're quoting Brian, I'll continue,
have a look at
http://concurrency.markmail.org/message/zts4ffuauk7gqq4q?q=running+with+scissors

Is there some reason that your constructors that start threads can't be
refactored into static factory methods that construct the instance then
start the thread?

For example,

class Foo ... {
  public Foo() {
    // init stuff
    // start thread
  }
}

becomes

class Foo ... {
  private Foo() {
    // init stuff
  }

  public static Foo getInstance() {
    final Foo fooInstance = new Foo();
    // start thread
    return fooInstance;
  }
}


> -Marco
>
>
> - Show quoted text -
>
> On Fri, Dec 10, 2010 at 5:52 PM, David Holmes <davidcholmes at aapt.net.au>
> wrote:
> > Hi Marco,
> >
> > There are some obviously silly things you can do with this pattern eg:
> >
> > class DontDoThis {
> >   Object field1;
> >   DontDoThis() {
> >     (new Thread() { public void run() {
> >          int x = field1.hashcode(); // potential NPE
> >          ...
> >      }}).start();
> >      field1 = new Object();
> >   }
> > }
> >
> > This the classic example of not being thread-safe, or rather of being
> racy.
> > The thread can access default initialized object-state.
> >
> > But if you start the thread at the end of the constructor there is
> nothing
> > that can go wrong for this class - there is a happens-before edge between
> > starting a thread and execution of its run method. So no races and no
> > visibility issues: all state is fully initialized.
> >
> > But if you then subclass that "safe" class you could again get racy
> > behaviour if the run method invokes a virtual method that is overridden
> in
> > the subclass and which accesses additional state - which can only be
> > initialized after the superclass constructor has run.
> >
> > For the most part though such constructs are not considered good design -
> > particularly when it comes to threading - because only the simplest of
> > designs can get away without some form of lifecycle control, for which it
> is
> > far better to separate construction of the service from the starting of
> the
> > service. There may be additional services that need to be configured
> before
> > this service should be started. (Imagine a car that automatically started
> > the engine as soon as you sat in the drivers seat and closed the door! -
> > occasionally convenient, but potentially quite hazardous)
> >
> > Given it is a poor design approach for a service, and it can easily
> > introduce race conditions, best practice says "don't do this".
> >
> > HTH
> >
> > David Holmes
> >
> >> -----Original Message-----
> >> From: concurrency-interest-bounces at cs.oswego.edu
> >> [mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of Marco
> >> Villalobos
> >> Sent: Saturday, 11 December 2010 11:30 AM
> >> To: concurrency-interest at cs.oswego.edu
> >> Subject: [concurrency-interest] Starting a Thread within a
> >> constructor,what can go wrong?
> >>
> >>
> >> QUESTION:
> >>
> >> Can somebody please give me guidance on how I could possibly create a
> >> suite of test cases that run in TestNG or JUnit that can prove the
> >> thread unsafeness of this bad practice?
> >>
> >> BACKGROUND
> >>
> >> I'm trying to convince my colleague that starting a thread within a
> >> constructor compromises thread safety.
> >>
> >> I understand that, "publishing objects before they are fully
> >> constructed can compromise thread safety."
> >>
> >> But I would like an elaboration on how it compromises thread safety?
> >>
> >> I understand that publishing the "this" reference would break
> >> encapsulation, allowing other classes and threads to potentially
> >> change the state of my object in an unthread safe manner.
> >>
> >> But more specifically, what are the side affects of an object not
> >> being fully constructed, yet published through a thread.start in a
> >> constructor.
> >>
> >> Here is an example of how "not to do things".
> >>
> >> public class DontDoThis {
> >>
> >>   public synchronized void mutateMe() {
> >>       // ... I'm mutating this...
> >>   }
> >>
> >>   public final Thread t;
> >>
> >>   public DontDoThis() {
> >>       t = new Thread(new Runnable() {
> >>           public void run() {
> >>               mutateMe();
> >>           }
> >>       });
> >>       t.start();   //     BAD BAD BAD Don't do this.
> >>   }
> >> }
> >>
> >> Can somebody please give me guidance on how I could possibly create a
> >> suite of test cases that run in TestNG or JUnit that can prove the
> >> thread unsafeness of this bad practice?
> >>
> >> I want to make a short presentation on the issue.  If I provide a
> >> compilable and running test case, that would be amazing :)
> >>
> >> -Marco
> >> _______________________________________________
> >> Concurrency-interest mailing list
> >> Concurrency-interest at cs.oswego.edu
> >> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> >>
> >
> >
>
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20101210/0787d3dd/attachment-0001.html>


More information about the Concurrency-interest mailing list