tillrohrmann commented on a change in pull request #13124:
URL: https://github.com/apache/flink/pull/13124#discussion_r469037667
##########
File path:
flink-core/src/main/java/org/apache/flink/util/AbstractCloseableRegistry.java
##########
@@ -163,9 +163,9 @@ protected final void addCloseableInternal(Closeable
closeable, T metaData) {
/**
* Removes a mapping from the registry map, respecting locking.
*/
- protected final void removeCloseableInternal(Closeable closeable) {
+ protected final boolean removeCloseableInternal(Closeable closeable, T
object) {
synchronized (getSynchronizationLock()) {
- closeableToRef.remove(closeable);
+ return closeableToRef.remove(closeable, object);
Review comment:
Why is `closeableToRef.remove(closeable)` not enough here? I think it
would not matter which `PhantomDelegatingCloseableRef` eventually removes the
`closeable` as long as it only gets closed once. The problem I see with the
current approach is that it complicates the API (e.g. the dev has to know what
`object` actually is).
----------------------------------------------------------------
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:
us...@infra.apache.org
