Re: The parts of the ReentrantReadWriteLock hold no reference to it

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: The parts of the ReentrantReadWriteLock hold no reference to it

Martin Grajcar
The parts of the ReentrantReadWriteLock holding no reference to it may lead to a situation where the ReentrantReadWriteLock gets GC'ed while its read and write locks are still in use. This is alright unless the ReentrantReadWriteLock is weakly cached as e.g., in Guava Striped, where it has lead to the bug https://github.com/google/guava/issues/2477.

I'd suggest to add a reference to the enclosing class, e.g., by removing the static modifier. As all these objects are lightweight and not meant to have millions of instances, the cost is negligible. The advantage is decreased chance of buggy user code and avoiding lengthy workarounds like https://github.com/google/guava/commit/957c1a5455508120d224f6d0d8f3bf8afa3630f0.

The cost benefit ratio seems to be good.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: The parts of the ReentrantReadWriteLock hold no reference to it

Martin Buchholz-3

On Tue, Oct 17, 2017 at 2:02 PM, Martin Grajcar <[hidden email]> wrote:
The parts of the ReentrantReadWriteLock holding no reference to it may lead to a situation where the ReentrantReadWriteLock gets GC'ed while its read and write locks are still in use. This is alright unless the ReentrantReadWriteLock is weakly cached as e.g., in Guava Striped, where it has lead to the bug https://github.com/google/guava/issues/2477.

I'd suggest to add a reference to the enclosing class, e.g., by removing the static modifier. As all these objects are lightweight and not meant to have millions of instances, the cost is negligible. The advantage is decreased chance of buggy user code and avoiding lengthy workarounds like https://github.com/google/guava/commit/957c1a5455508120d224f6d0d8f3bf8afa3630f0.

The cost benefit ratio seems to be good.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: The parts of the ReentrantReadWriteLock hold no reference to it

Doug Lea
In reply to this post by Martin Grajcar
On 10/17/2017 05:02 PM, Martin Grajcar wrote:
> The parts of the ReentrantReadWriteLock holding no reference to it may
> lead to a situation where the ReentrantReadWriteLock gets GC'ed while
> its read and write locks are still in use. This is alright unless
> the ReentrantReadWriteLock is weakly cached as e.g., in Guava Striped,
> where it has lead to the bug https://github.com/google/guava/issues/2477
> <https://github.com/google/guava/issues/2477>.

>
> I'd suggest to add a reference to the enclosing class, e.g., by removing
> the static modifier. As all these objects are lightweight and not meant
> to have millions of instances, the cost is negligible. The advantage is

Some program out there probably has more ReentrantReadWriteLocks than
any of use can imagine, so will be hurt by this.
Still, I suppose we have done worse things to help solve
not-our-problem problems in even more crazy uses than yours,
so we probably ought to do it.

Maybe the added overhead will drive more people to use StampedLocks,
as we keep hoping.

-Doug

> decreased chance of buggy user code and avoiding lengthy workarounds
> like https://github.com/google/guava/commit/957c1a5455508120d224f6d0d8f3bf8afa3630f0
> <https://github.com/google/guava/commit/957c1a5455508120d224f6d0d8f3bf8afa3630f0>.
>
> The cost benefit ratio seems to be good.
>
>
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest