poorbarcode commented on code in PR #22858:
URL: https://github.com/apache/pulsar/pull/22858#discussion_r1629184307
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java:
##########
@@ -982,6 +1000,10 @@ public CompletableFuture<ClientCnx> getConnection(final
String topic, final Stri
public LookupService getLookup(String serviceUrl) {
return urlLookupMap.computeIfAbsent(serviceUrl, url -> {
+ if (isClosed()) {
+ throw new IllegalStateException("Pulsar client has been
closed, can not build LookupService when"
+ + " calling get lookup with an url");
+ }
Review Comment:
> I don't think it's actual a memory leak. This PR only clears the map after
closeAsync is called. However, I could hardly think of a case that a
PulsarClient is still used after closeAsync. When the PulsarClientImpl object
is GC'ed, this map will be GC'ed as well.
- The map obj is not an important point.
- `LookupService` is a child class of `Closable`, which means it always
holds resources (such as connections or other resources). Before lookup
services have been GC, should call `close`.
> Move it before the `computeIfAbsent`.
In the method `computeIfAbsent` is better, because there is a case like
below:
| `thread-1`| `thread-2` |
| --- | --- |
| | check state: `open` |
| check state -> closing |
| for-each: close |
| | computeIfAbsent |
The new lookup service created through `thread-2` will not be released.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
