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

[Chris Dennis]

(Sorry for mixing top and bottom posting - hope nobody is offended!)

You'll need to blank the cachedHoldCounter when a thread releases it's
last hold as well, right?  Otherwise we still leak a reference to the last
reader to take this lock, even if that thread has unlocked fully.

On 6/25/13 4:09 PM, "Aleksey Shipilev" <aleksey.shipilev at oracle.com> wrote:

>On 06/25/2013 11:54 PM, Chris Dennis wrote:
>> In a similar vein, the following code doesn't behave like you'd expect
>> either (although it's arguable as to whether this is a reasonable way
>>for
>> RRWL to behave in this circumstance).  Note here that this is totally
>> legal per the getId() contract, because the id doesn't get recycled
>>until
>> after the join returns.
>
>Now that's a good example!
>
>This highlights the potential problem in RRWL. The code should instead
>throw the IllegalMonitorStateException in t2b.run(). I wonder what harm
>is done if we disregard the comment there and store the reference to
>Thread instead.
>
>        /**
>         * A counter for per-thread read hold counts.
>         * Maintained as a ThreadLocal; cached in cachedHoldCounter
>         */
>        static final class HoldCounter {
>            int count = 0;
>            // Use id, not reference, to avoid garbage retention
>            final long tid = Thread.currentThread().getId();
>        }
>
>        static final class ThreadLocalHoldCounter
>            extends ThreadLocal<HoldCounter> {
>            public HoldCounter initialValue() {
>                return new HoldCounter();
>            }
>        }
>
>I don't see the problem off-hand: if thread dies, so do the associated
>ThreadLocalMap-s. RRWL does not reference the live
>ThreadLocalHoldCounter when nobody is holding the read lock... What else
>is missing?
>
>-Aleksey.