3

Not sure whether this D class thread is correct. Is there a race condition, is i supposed to be in a synchronized block when accessed? What if D is an outer class, and the A instance was passed to a D constructor?

class A
{
    int i;
    void f() {
        i++;
        D d = new D();
        d.start();
    }
    class D extends Thread {
        public void run() {
            int g = i;
        }
    }
}
Cory Kendall
  • 7,195
  • 8
  • 37
  • 64
Sam Adamsh
  • 3,331
  • 8
  • 32
  • 53
  • 2
    Completely unrelated, as the others seem to have you covered, but it is considered bad practice to extend `Thread`. – pickypg May 08 '13 at 06:13
  • Google devs do it all the time in android sample code! – Sam Adamsh May 08 '13 at 06:14
  • 3
    Here's the [better approach](http://stackoverflow.com/questions/1921514/how-to-run-a-runnable-thread-in-android). This way you cannot find yourself accidentally overriding things, nor are you carrying a lot of extra memory baggage that you don't need. – pickypg May 08 '13 at 06:19
  • As is, no issue at all. You increment `i` before you start the thread and no thread modifies `i` at all. – xagyg May 08 '13 at 06:31

3 Answers3

1

Is there a race condition?

Possibly if you call f from more than one thread as you would be accessing a shared variable (i) without proper synchronization.

Note however that starting a thread creates a happens-before relationship.

So g will be equal to the value of i right before the thread was started (i.e. the value of i when d.start() is called) or any subsequent values if i has been modified by another thread in the meantime (with no guarantee that such modifications are actually seen).

assylias
  • 321,522
  • 82
  • 660
  • 783
1

This is safe as long as you only invoke f once. There is a happens-before relationship between a thread A that mutates data and a thread B that's started from thread A (the HB-relationship being at Thread.start). Since nobody mutates data after D is started, this is safe.

Some ways to break the thread safety:

  • mutate i again, including by invoking foo again
  • read i from a thread other than D or the one that invoked foo

The reason that you can't mutate i again, even from the thread that invoked foo, is that this mutation would have happened after d.start(), and would therefore have no HB edge against that second mutation.

The reason you can't read i from any arbitrary thread is that this thread wouldn't have a well-defined view of the i++ mutation.

It can get a tad more subtle than that, but at a high level, there you go.

yshavit
  • 42,327
  • 7
  • 87
  • 124
1

If f() will be called from one and the same thread there will be no problem because start() guarantees that i is visible from run() and run() does not change i. Otherwise you need AtomicInteger or synchronized. Note that simply volatile i will not help, some i++ may be lost.

Evgeniy Dorofeev
  • 133,369
  • 30
  • 199
  • 275