river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patricia Shanahan <p...@acm.org>
Subject Re: ThreadPool calling Thread.setName
Date Sat, 27 Apr 2013 15:43:03 GMT
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.
>>>>
>>>
>>
>


Mime
View raw message