how to make this code thread safer?
in [Java Help]
|
From: Peter Duniho on 20 Jul 2010 18:26 Alan Gutierrez wrote: > [...] >> If you like, we can just copy/paste the back-and-forth we had there to >> this newsgroup. > > I'd be interested to see that. I'd tend to agree with Arne. I generally > avoid holding the monitor on an object whose class is not part of the > code base, that is, I know that there is a monitor on every object, so I > don't try to hold the monitor on an instance someone else's class, > because that there for them to use. > > The encapsulation as proscribed by the C# "best-practice" makes it > impossible, I understand, but I really cotton to Arne's comment on > "best-practices". The C# "best-practice" recommendation is to not lock on "this". Following that recommendation in no way prevents _other_ code from locking on the object reference itself. In fact, there is no way to prevent other code from locking on the object reference itself, short of acquiring that lock and never releasing it. Hence the recommendation for a class to not lock on its own "this" reference. There's no way to know what other code might be using that reference for a lock. > So, other than the ounce of prevention, what makes Arne wrong? (Or, link > to the previous discussion if you're not interested.) Here's the link: http://groups.google.com/group/microsoft.public.dotnet.languages.csharp/browse_thread/thread/95fa9dbd85a37227/dd03736b7038541c As far as my point of view goes, I find the advice useful for the same reason I find the general OOP philosophy of information hiding and encapsulation useful. There are a number of mistakes I have successfully avoided even before I was using OOP languages and which are prevented or at least minimized through the use of good OOP practices. But the fact that I've never actually had or seen those problems doesn't mean I should eschew OOP best-practices. Should I insist that I make and/or observe every possible mistake possible before I incorporate policies in my own code to avoid those mistakes? You may find these discussions interesting as well: http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java http://www.yoda.arachsys.com/csharp/threads/lockchoice.shtml The former is not in any way 100% pro-"don't lock on this", but it has a bit of reasonable back-and-forth. It also includes a (perhaps contrived) code example of how using "this" for synchronization not only can lead to problems, it can expose code to an intentional denial-of-service attack (not necessarily something that is even possible in all scenarios, but certainly worth thinking about IMHO). Pete
From: Alan Gutierrez on 20 Jul 2010 20:45 Peter Duniho wrote: > Alan Gutierrez wrote: >> [...] >>> If you like, we can just copy/paste the back-and-forth we had there >>> to this newsgroup. >> >> I'd be interested to see that. I'd tend to agree with Arne. I >> generally avoid holding the monitor on an object whose class is not >> part of the code base, that is, I know that there is a monitor on >> every object, so I don't try to hold the monitor on an instance >> someone else's class, because that there for them to use. >> >> The encapsulation as proscribed by the C# "best-practice" makes it >> impossible, I understand, but I really cotton to Arne's comment on >> "best-practices". > > The C# "best-practice" recommendation is to not lock on "this". > Following that recommendation in no way prevents _other_ code from > locking on the object reference itself. > > In fact, there is no way to prevent other code from locking on the > object reference itself, short of acquiring that lock and never > releasing it. Hence the recommendation for a class to not lock on its > own "this" reference. There's no way to know what other code might be > using that reference for a lock. > >> So, other than the ounce of prevention, what makes Arne wrong? (Or, >> link to the previous discussion if you're not interested.) > > Here's the link: > http://groups.google.com/group/microsoft.public.dotnet.languages.csharp/browse_thread/thread/95fa9dbd85a37227/dd03736b7038541c > > > As far as my point of view goes, I find the advice useful for the same > reason I find the general OOP philosophy of information hiding and > encapsulation useful. There are a number of mistakes I have > successfully avoided even before I was using OOP languages and which are > prevented or at least minimized through the use of good OOP practices. > But the fact that I've never actually had or seen those problems doesn't > mean I should eschew OOP best-practices. Thank you for taking the time to layout a lesson plan. I read it all. Yes, it is truly an ounce of prevention, and an appropriate application of encapsulation. Somewhere else in this list, someone wrote, objects are cheap, defects are expensive. Create a mutex Object makes sense. The Collections.concurrentMap implementation is probably considered by C# programmers to be too clever. -- Alan Gutierrez - alan(a)blogometer.com - http://twitter.com/bigeasy
From: Lew on 21 Jul 2010 01:09 markspace wrote: > I agree that the only practical way to pass an object to the EDT > involves a shared lock. However, invokeLater() makes no such guarantee, Are you kidding? The whole documented point of 'invokeLater()' and 'invokeAndWait()' is to push things to the EDT in a thread-safe manner. > 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 They totally would not be within their rights to do that, as it would violate the entire point of the method. When you look at the source and see all the work they did to build synchronization into those methods, you will understand that it's there for a reason - to do what the methods freaking promise to do! > docs of some of the concurrent classes like Executor, which does provide > an explicit memory consistency guarantee. I agree that the promise should be repeated in the Javadocs for the methods themselves, but I'm not going to bet that anyone's going to remove synchronization from the implementation of the 'invoke...()' methods. -- Lew
From: markspace on 21 Jul 2010 01:56 Lew wrote: > I agree that the promise should be repeated in the Javadocs for the > methods themselves, This is my main point. If that is indeed their intention, they should document it. The Swing team needs a once-over with a wet noodle for some of their concurrency documentation.
From: markspace on 21 Jul 2010 02:27
Lew wrote: > markspace wrote: >> I agree that the only practical way to pass an object to the EDT >> involves a shared lock. However, invokeLater() makes no such guarantee, > > Are you kidding? The whole documented point of 'invokeLater()' and > 'invokeAndWait()' is to push things to the EDT in a thread-safe manner. Addendum: no, I'm not kidding. Where do you see it documented that the purpose of invokeLater() is to "push things" to the EDT? I only see it documented that invokeLater() executes code, so that already existing objects, created on the EDT, can be accessed in a thread safe manner. All the examples of using invokeLater() I can find carefully avoid any access of objects created off the EDT, unless that object is made thread safe some other way (e.g., use of final keyword). If you can show documentation that even partially supports your claim, I'd love to see it. |