You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/02/10 23:19:47 UTC

[GitHub] [accumulo] ctubbsii opened a new issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

ctubbsii opened a new issue #1924:
URL: https://github.com/apache/accumulo/issues/1924


   **Describe the bug**
   While looking at our use of WeakHashMap throughout our code, I observed that the SharedRateLimiterFactory uses a WeakHashMap called `activeLimiters`. However, it uses `activeLimiters.put(name, new SharedRateLImiter(name, ....))`, and the constructor stores a strong reference to `name` as a member variable. Therefore, it is impossible for `activeLimiters` to ever garbage collect and remove references once added.
   
   **Versions (OS, Maven, Java, and others, as appropriate):**
    - Affected version(s) of this project: 2.1.0-SNAPSHOT (probably others... only looked at this one)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1924:
URL: https://github.com/apache/accumulo/issues/1924#issuecomment-796396470


   @DomGarguilo The `WeakHashMap` already uses weakly referenced keys. However, the objects in the value are strong references by default. The way to ensure the values in the hashmap don't retain strong references to the keys in the map is to wrap the values themselves in a `WeakReference`. So, instead of a `WeakHashMap<String,SharedRateLimiter>`, you would simply wrap the values and make them `WeakHashMap<String,WeakReference<SharedRateLimiter>>` instead.  See the implementation note in the [`WeakHashMap` javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/WeakHashMap.html) for a similar explanation.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1924:
URL: https://github.com/apache/accumulo/issues/1924#issuecomment-777802398


   > > the constructor stores a strong reference to `name` as a member variable
   > 
   > Which constructor are you referring to?
   
   `new SharedRateLImiter`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii closed issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

Posted by GitBox <gi...@apache.org>.
ctubbsii closed issue #1924:
URL: https://github.com/apache/accumulo/issues/1924


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #1924:
URL: https://github.com/apache/accumulo/issues/1924#issuecomment-777797567


   > the constructor stores a strong reference to `name` as a member variable
   
   Which constructor are you referring to?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on issue #1924: SharedRateLimiterFactory.create() method does not correctly use WeakHashMap

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #1924:
URL: https://github.com/apache/accumulo/issues/1924#issuecomment-795644434


   From what I can tell, there is no easy way to get this to work while using `WeakHashMap`. It seems the strong reference to `name` in `SharedRateLimiter` might be unavoidable. Maybe the best option is just to use `HashMap`.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org