how to make this code thread safer?
in [Java Programming]
|
From: markspace on 19 Jul 2010 15:31 Peter Duniho wrote: > The first mechanism has to do with semantics surrounding object > construction. In .NET, the specification ensures that whatever happens > in the constructor "happens before" any other code that uses the > reference after the constructor completes (.NET doesn't actually use the > term "happens-before", but it's the same idea). I would expect Java to > be the same in this respect. I'm pretty sure it's not the same in Java. The semantics of the constructor only guarantee that an object will be visible to the calling thread. All other threads may not see a completely and correctly constructed object. And I'd be surprised if it's any different in C#, because that would imply a memory barrier at every object construction, although I guess C# could indeed be doing such a thing. > In the case of the EDT, the typical way an object reference would get > from another thread to the EDT is via the invokeLater() or > invokeAndWait() methods. These methods are inherently synchronized > themselves; it's a natural consequence of what must happen for the data > passed to the methods to get to the EDT. I agree that the only practical way to pass an object to the EDT involves a shared lock. However, invokeLater() makes no such guarantee, and they'd be within their rights to change their implementation of invokeLater() and remove any shared locks, thus removing any synchronization and any happens-before semantics. Contrast this with the docs of some of the concurrent classes like Executor, which does provide an explicit memory consistency guarantee. Thus, I think the minimum you can get away with is a volatile field: public class Main { public static void main( String... args ) { final MyClass test = new MyClass(); SwingUtilities.invokeLater( new Runnable() { private volatile MyClass temp = test; public void run() { MyClass model = temp; ... // ok to use model now... } } ); } } > This applies more generally to any cross-thread communication mechanism. > Even if not using the EDT, there is likely some synchronized > data-transfer mechanism that acts as the go-between, allowing that newly > constructed reference to safely move from one thread to the other. This is an even scarier assertion, imo. I don't agree that we should rely on what's "likely" to happen in concurrency, unless you only want it "likely" that your program will run correctly.
From: Peter Duniho on 19 Jul 2010 15:45 markspace wrote: > [...] >> In the case of the EDT, the typical way an object reference would get >> from another thread to the EDT is via the invokeLater() or >> invokeAndWait() methods. These methods are inherently synchronized >> themselves; it's a natural consequence of what must happen for the >> data passed to the methods to get to the EDT. > > > I agree that the only practical way to pass an object to the EDT > involves a shared lock. However, invokeLater() makes no such guarantee, > and they'd be within their rights to change their implementation of > invokeLater() and remove any shared locks, thus removing any > synchronization and any happens-before semantics. I disagree. Java concurrency and especially that related to the EDT (that is, in light of the fact that EDT-related issues are almost always about concurrency) is poorly documented, I will grant. But it is simply not possible for the invoke methods to not provide the necessary synchronization. Whatever has happened in one thread prior to the call to an invoke method _must_ be visible to the code executing as a result of that call to the invoke method. The invoke methods would be useless otherwise. > Contrast this with > the docs of some of the concurrent classes like Executor, which does > provide an explicit memory consistency guarantee. > > Thus, I think the minimum you can get away with is a volatile field: > > public class Main { > public static void main( String... args ) { > final MyClass test = new MyClass(); > SwingUtilities.invokeLater( new Runnable() { > private volatile MyClass temp = test; > public void run() { > MyClass model = temp; > ... // ok to use model now... > } > } ); > } > } If what you say were true, then you could not even count on the Runnable instance that you created to be valid during the invoked execution, since that's something that happened before the call to invokeLater(), but which wasn't synchronized. The "temp" field itself would be safe, but nothing else in the Runnable instance would be (including, for example, a v-table as might be used to dispatch the call to run()). >> This applies more generally to any cross-thread communication >> mechanism. Even if not using the EDT, there is likely some >> synchronized data-transfer mechanism that acts as the go-between, >> allowing that newly constructed reference to safely move from one >> thread to the other. > > This is an even scarier assertion, imo. I don't agree that we should > rely on what's "likely" to happen in concurrency, unless you only want > it "likely" that your program will run correctly. You are misreading my statement. It's not about relying on "what's likely". It's that it is "likely" there is a synchronization mechanism in place, and if so one can rely on it. You can certainly move data from one thread to another unsafely. For example, through use of a non-volatile field. And yes, even if the data managed to get from one thread to the other via that field, there would be no guarantees about other data that may have been updated in one thread before the data in question, being visible in the other thread even though the data in question is visible. But, assuming the data itself was moved safely (e.g. through use of a volatile field), then all of the things that happened in one thread before that data was moved from that thread will then be visible in another thread after the data is moved to that thread. And since for the code to be thread-safe, the data _must_ be moved safely, it is "likely" that the data was moved safely (inasmuch as one hopes it is "likely" that the code is thread-safe). The likeliness depends of course on how much you trust the person who wrote the code moving the data. But for mechanisms found in the JDK, I find the level of likelihood quite high. For a random Java programmer I stumble across on the street, not so much. Pete
From: Peter Duniho on 19 Jul 2010 15:57 Peter Duniho wrote: > [...] >> Thus, I think the minimum you can get away with is a volatile field: >> >> public class Main { >> public static void main( String... args ) { >> final MyClass test = new MyClass(); >> SwingUtilities.invokeLater( new Runnable() { >> private volatile MyClass temp = test; >> public void run() { >> MyClass model = temp; >> ... // ok to use model now... >> } >> } ); >> } >> } > > If what you say were true, then you could not even count on the Runnable > instance that you created to be valid during the invoked execution, > since that's something that happened before the call to invokeLater(), > but which wasn't synchronized. The "temp" field itself would be safe, > but nothing else in the Runnable instance would be (including, for > example, a v-table as might be used to dispatch the call to run()). Sorry, just to be clear: I overstate when I use the word "nothing". But, only those things that occurred prior to the initialization of the "volatile" field would be guaranteed to be visible in the EDT. Other initialization of the object would not be. I would guess that things like a v-table would be initialized before the field, but since you can't reference "this" in an initializer, even that may be open to question. Pete
From: Daniel Pitts on 19 Jul 2010 16:32 On 7/19/2010 9:33 AM, www wrote: > The code below is completely fine if running on single-thread > environment. But it could break if running on multi-thread environment. > I have added my analysis as comments in the code to show my > understanding why it is not thread safe. My real code, with a pattern > like the code shown here, indeed breaks once in a while with Null > Pointer Exception in multi thread run. Do you think my analysis is > correct? Can you help me on making this code thread safer? There are a few ways to handle this, the most obvious approach would be to synchronize the whole thing, which in essences prevents the code from running in a multi-threaded manor. I suggest reading the book Java Concurrency in Practice. <http://www.amazon.com/dp/0321349601?tag=virtuinfinnet-20&camp=14573&creative=327641&linkCode=as1&creativeASIN=0321349601&adid=1Z4QATYV394TGJNXFGXC&> "This book is a must read for anyone who works in a multi-threaded application, which turns out is a lot more frequently than you might expect. If you work in a J2EE container, Swing GUI application, or any kind of concurrent system, this book will give you the know-how to create efficient, scalable, and correct programs. It helps dispel common misconceptions about concurrency, and showcases the improvements to concurrent programming in Java 5." -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
From: Joshua Cranmer on 19 Jul 2010 17:33
On 07/19/2010 02:51 PM, Peter Duniho wrote: > The first mechanism has to do with semantics surrounding object > construction. In .NET, the specification ensures that whatever happens > in the constructor "happens before" any other code that uses the > reference after the constructor completes (.NET doesn't actually use the > term "happens-before", but it's the same idea). I would expect Java to > be the same in this respect. This is only true for final fields. Regular fields do not have the same luxury--hence why it is often preferred to have immutable objects in Java. -- Beware of bugs in the above code; I have only proved it correct, not tried it. -- Donald E. Knuth |