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

Nathan Reynolds nathan.reynolds at oracle.com
Tue Jun 25 14:33:08 EDT 2013


I have filed bug https://jbs.oracle.com/bugs/browse/JDK-8017617 for this.

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 11:30 AM, Chris Dennis wrote:
> 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 
> <mailto: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 
>> <mailto: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/concurrency-interest
>>
>> _______________________________________________ Concurrency-interest 
>> mailing list Concurrency-interest at cs.oswego.edu 
>> <mailto: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/292ab5d1/attachment.html>


More information about the Concurrency-interest mailing list