gperinazzo commented on issue #62: Listening for messages URL: https://github.com/apache/pulsar-client-node/issues/62#issuecomment-612976617 @yosiat I'm seeing a crash if a client with listener is closed twice after your pull request: ```js const Pulsar = require('./index.js'); const delay = (time) => new Promise(resolve => setTimeout(resolve, time)); (async () => { // Create a client const client = new Pulsar.Client({ serviceUrl: 'pulsar://localhost:6650' }); let count = 0; // Create a consumer const consumer = await client.subscribe({ topic: 'persistent://public/default/my-topic', subscription: 'sub1', subscriptionType: 'Shared', ackTimeoutMs: 10000, listener: function(msg, consumerArg) { console.log(new Date().toLocaleString(), msg.getData().toString()); delay(1000).then(() => { consumerArg.acknowledge(msg); consumerArg.close(); }) } }); })(); ``` Crashes with the following output: ``` FATAL ERROR: Error::New napi_get_last_error_info 1: 0x555d20446711 node::Abort() [node] 2020-04-13 13:04:10.834 INFO ConsumerImpl:894 | [persistent://public/default/my-topic, sub1, 0] Closed consumer 0 2: 0x555d20386273 node::FatalError(char const*, char const*) [node] 3: 0x555d2038627c [node] 4: 0x555d20416d11 napi_fatal_error [node] 5: 0x7f7ab97e6f1a [/home/guilherme/Projects/pulsar-client-node/build/Release/Pulsar.node] 6: 0x7f7ab982af2a Consumer::Close(Napi::CallbackInfo const&) [/home/guilherme/Projects/pulsar-client-node/build/Release/Pulsar.node] 7: 0x7f7ab98313c6 Napi::ObjectWrap<Consumer>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*) [/home/guilherme/Projects/pulsar-client-node/build/Release/Pulsar.node] 8: 0x555d203fbf05 [node] 9: 0x555d2062b657 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [node] 10: 0x555d2062ba21 [node] 11: 0x555d2062c2da [node] 12: 0x555d2062cbf6 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node] 13: 0x555d20db6e99 [node] Aborted (core dumped) ``` Before the PR, the second call to Close would fail with an Already Closed error, but without crashing Node. I wrote the original listener implementation, thanks for cleaning it up! Most of the reason it was messy was that I couldn't find a better way to not crash and not leak any memory. It's really annoying due to how the pulsar C interface was made. The problem is that the C interface creates a `pulsar_consumer_t` on the stack and passes it to the message listener ([here](https://github.com/apache/pulsar/blob/master/pulsar-client-cpp/lib/c/c_ConsumerConfiguration.cc#L52)). The consumer it receives is a shared pointer, but the one that is passed to the message listener can't be used after the original message listener function has returned due to this. I think the golang client wrapper workaround for this leaks memory, but I'm not sure. It saves a pointer to the consumer and channel to be used by the listener but I can't find where it deletes the pointer. Maybe we could add a function to the C interface that clones a `pulsar_consumer_t`, creating a new shared pointer to the same C++ consumer, and that has to be freed manually? This would allow libraries like this to clone the consumer received by the message listener callback to use it later to ack the message. That would simplify wrapper libraries a lot.
---------------------------------------------------------------- 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 With regards, Apache Git Services
