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

Chris Hegarty chris.hegarty at oracle.com
Tue Jun 25 15:19:19 EDT 2013


On 06/25/2013 07:33 PM, Nathan Reynolds wrote:
> I have filed bug https://jbs.oracle.com/bugs/browse/JDK-8017617 for this.

The public bug for this will available, soon, at:
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8017617

If there is agreement that getId() is fundamentally flawed and cannot be 
depended upon, even for internal JDK implementation, then I see this as 
pretty good justification to add a new final method to return the 
stable/final Id.

I can float this on the OpenJDK mailing lists and see if there are any 
objections.

-Chris.

>
> 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
>>
>
>
>
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>


More information about the Concurrency-interest mailing list