Based on your reasoning and because all tests are passing reliably without failure now, it should be safe to uncomment it.
Peter. ----- Original message ----- > I also don't see the particular concern with toString(). It uses > getName(), so it does all its work with a particular char[]. At the > worst, that char[] will either be an old one, or the current one but > with some out-of-date elements, resulting in an unhelpful value. > > Patricia > > > On 4/27/2013 5:55 AM, Patricia Shanahan wrote: > > I'll read the paper. However, C and C++ tend to have nastier semantics > > for data races than Java, which lacks anything quite as drastic as the C > > definition of "undefined behavior". > > > > My reasoning for the relative safety of this particular race is based on > > one of the more basic rules, atomicity of reference read or write, > > combined with reading the relevant code. Regardless of data races, the > > result of accessing "name" will always be a pointer to a char[]. We have > > a significant chance of the char[] containing useful, helpful data in > > the old version of the code. The probability of it containing anything > > useful is zero in the new version. > > > > Patricia > > > > > > On 4/27/2013 3:51 AM, Peter Firmstone wrote: > > > Ok found the link, the paper's called: > > > Position paper: Nondeterminism is unavoidable, but data races are > > > pure evil > > > > > > http://www.hpl.hp.com/techreports/2012/HPL-2012-218.pdf > > > > > > > > > Wheather Thread's name field is an issue or not depends on the code that > > > uses it (Object.toString() and Thread.getName() methods do) and what > > > that code does with the information. > > > > > > Outrigger uses it in lifecycle logs, although it doesn't appear to be > > > called on any threads from ThreadPool. Instead Outrigger calls > > > getName() on threads it references from fields in OutriggerServerImpl. > > > These fields have since been changed to final, so if the name is set > > > during construction and not changed after publication, it's thread safe, > > > in addition these threads aren't started until after > > > OutriggerServerImpl's constructor returns. > > > > > > The concurrency issues I most recently experienced were related to > > > Outrigger, I couldn't determine the cause of these failures, perhaps I > > > lack sufficient ability to reason deeply about these issues, my fix for > > > the problem was akin to carpet bombing; fix every concurrency problem > > > and that appears to have paid off. > > > > > > It's very difficult to determine if toString() may be called on > > > ThreadPool threads. I reasoned it could be and that was sufficient > > > justification to not use setName(). > > > > > > Cheers, > > > > > > Peter. > > > > > > > > > > > > > > > > > > > > > On 27/04/2013 1:20 AM, Patricia Shanahan wrote: > > > > I've looked at the source code. The field "name" that is shared > > > > between threads doing getName or setName on a given Thread is a > > > > reference. Writes and reads of references are always atomic. > > > > > > > > The worst that could happen is that the change to the name does not > > > > propagate to all threads that might display information about the > > > > thread. The proposed fix ensures that worst case outcome happens all > > > > the time. > > > > > > > > Patricia > > > > > > > > On 4/26/2013 5:44 AM, Greg Trasuk wrote: > > > > > > > > > > I'm curious, as I don't see any indication in the Javadocs that > > > > > setName() isn't thread safe. Is there another reference that calls > > > > > that > > > > > out? And what would be the failure mode, apart from a mangled > > > > > string in > > > > > a log output? > > > > > > > > > > Personally, if the potential failure mode wasn't onerous, I'd opt for > > > > > more descriptive logging. Comprehensibility is everything when you're > > > > > troubleshooting. > > > > > > > > > > Cheers, > > > > > > > > > > Greg. > > > > > > > > > > On Fri, 2013-04-26 at 05:48, Peter Firmstone wrote: > > > > > > Hope you don't mind, I've removed the call to Thread.setName in > > > > > > com.sun.jini.ThreadPool > > > > > > > > > > > > As a result threads will be less descriptive, unfortunately setName > > > > > > isn't thread safe, it's final and cannot be overridden. > > > > > > Thread.getName > > > > > > is only thread safe if a Thread's name isn't changed after > > > > > > publication. > > > > > > > > > > > > ThreadPool was the only instance of Thread.setName in River. > > > > > > > > > > > > Regards, > > > > > > > > > > > > Peter. > > > > > > > > > > > > > > >
