[concurrency-interest] RRWL with 'bad' Thread.getId() implementations

Chris Dennis chris.w.dennis at gmail.com
Tue Jun 25 14:30:15 EDT 2013


Yes, sadly people will insist on hanging themselves whenever we give them
enough rope.  I don't think I've actually seen someones code that did this,
but I have seen it's fallout:
http://forums.terracotta.org/forums/posts/list/0/2116.page

Something like getStableId() would seem like a good idea, although I
strongly suspect this debate has occurred within Sun/Oracle and there are
probably good technical reasons why it's not happened yet.  What I would
like to see as a minimum is someone adding some stronger wording to the
Javadoc for the method.

Chris

On 6/25/13 2:02 PM, "Nathan Reynolds" <nathan.reynolds at oracle.com> wrote:

    
 Wow!  Final getId() would break user code?  Interesting.  Are there any
examples?  I can dream up a few but I am curious what is out in the wild.
 
 I guess we need another method in Thread which is declared final and
returns the "tid" member variable value.  Maybe it could be called
getFinalId(), getImmutableId(), getFirmId(), or getStableId().
 

 
Nathan Reynolds 
<http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>  | Architect
| 602.333.9091
 Oracle PSR Engineering <http://psr.us.oracle.com/>  | Server Technology
 
 On 6/25/2013 10:56 AM, Chris Dennis wrote:
 
 
>  
> The issue about getId() not being final has been thrown about forever – it's
> not going to be fixed because it would cause too much breakage in user code.
> Seems strange to me though given that we know the non finality of getId allows
> users to do this, we aren't avoiding relying on that method – why don't we
> have some other way for library code (JDK or third-party) to get access to a
> unique identifier for a thread?
>  
> 
>  
>   
>  
> On 6/25/13 1:24 PM, "Nathan Reynolds" <nathan.reynolds at oracle.com> wrote:
>  
>  
> 
>  
>  
>   
>  
> The JavaDoc for Thread.getId() says "...thread ID is unique..." so I don't
> think this is a bug in RRWL.  Maybe Thread.getId() should be declared final.
>  
>  We might want to consider going as far as declaring the member field "tid" as
> final.  This could be done via "private long tid = nextThreadID();"
>  
>  I find it very interesting that threadInitNumber and threadSeqNumber are both
> used in the Thread class.  It seems we only need 1.  It seems that the
> constructor should use "Thread-" + tid for a thread name.  In fact, the name
> could read "Thread-10" and the tid could be 7 because there is a race between
> when the name is generated and the tid is set.  The mismatch probably doesn't
> matter functionally.  However, it could make it easier for debugging.
>  
>  
> Nathan  Reynolds 
> <http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>  | Architect |
> 602.333.9091
>  Oracle PSR Engineering <http://psr.us.oracle.com/>  | Server Technology
>  
>  On 6/25/2013 9:50 AM, Chris Dennis wrote:
>  
>  
>>  
>> Hi All,
>> 
>> While dealing with a customer issue, I ran in to the possibility of
>> breaking a RRWL by feeding in Thread instances with colliding thread-ids.
>> Inside RRWL the cachedHoldCounter logic assumes that getId() will return a
>> unique identifier for a thread.  Do people consider this a bug in RRWL or
>> not? (I think it would be agreed that this is also a bug in the
>> subclassing of Thread)
>> 
>> Regards,
>> 
>> Chris Dennis
>> 
>> public static void main(String[] args) {
>>   final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
>>   final CyclicBarrier barrier = new CyclicBarrier(2);
>>     
>>   Thread t1 = new EvilThread() {
>>     public void run() {
>>       try {
>>         lock.readLock().lock();
>>         barrier.await();
>>         //T2 locks
>>         barrier.await();
>>         //T3 locks
>>         //T3 unlocks
>>         barrier.await();
>>         //T2 unlocks
>>         barrier.await();
>>         lock.readLock().unlock();
>>       } catch (Exception e) {
>>         e.printStackTrace();
>>       }
>>     }
>>       
>>   };
>>   Thread t2 = new EvilThread() {
>>     public void run() {
>>       try {
>>         //T1 locks
>>                   barrier.await();
>>                   lock.readLock().lock();
>>                   barrier.await();
>>                   //T3 locks
>>                   //T3 unlocks
>>                   barrier.await();
>>                   lock.readLock().unlock();
>>                   barrier.await();
>>                   //T1 unlocks
>>       } catch (Exception e) {
>>         e.printStackTrace();
>>       }
>>     }
>>       
>>   };
>>   Thread t3 = new EvilThread() {
>>     public void run() {
>>       try {
>>         //T1 locks
>>         barrier.await();
>>                   //T2 locks
>>                   barrier.await();
>>                   lock.readLock().lock();
>>                   lock.readLock().unlock();
>>                   barrier.await();
>>                   //T2 unlocks
>>                   barrier.await();
>>                   //T3 unlocks
>>       } catch (Exception e) {
>>         e.printStackTrace();
>>       }
>>     }
>>       
>>   };
>>     
>>   t1.start();
>>   t2.start();
>>   t3.start();
>>   }
>> 
>>   
>>   static class EvilThread extends Thread {
>>   public long getId() {
>>     return 42L;
>>   }
>>   }
>> 
>> 
>> _______________________________________________
>> Concurrency-interest mailing list
>> Concurrency-interest at cs.oswego.eduhttp://cs.oswego.edu/mailman/listinfo/concu
>> rrency-interest
>>  
>  
>  
>  
>  _______________________________________________ Concurrency-interest mailing
> list Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
 
 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20130625/6b5ca73e/attachment-0001.html>


More information about the Concurrency-interest mailing list