Matt-Esch opened a new pull request #200: URL: https://github.com/apache/pulsar-client-node/pull/200
I have replaced the use of async workers with direct async functions by utilising a thread safe deferred (promise). The need for this comes from the fact that with workers, calling producer.send in order results in non-deterministic ordering, and awaiting every send would diminish the utility of having a batched mode. I also found that closing the client while worker threads were ready to execute resulted in segfaults, so this change ensures that most operations execute in the order they are called on the js side. I've also found several places lacking in good memory management discipline and have patched a few leaks. I've opted to use shared pointers with custom destructors to wrap all of the returned pointers, referring the pulsar library code to check who owns these pointers. (I did not change Auth but this could also benefit). I have avoided excessively adding std::move manually as modern compilers are quite good at optimising this themselves, but I would welcome suggestions where appropriate. I've changed the logger so that it is set globally. Currently the logger is supplied by the client configuration, but this gives the impression that the logger can be set per client. This is not true and the logger can only be set once for the lifetime of the process. The logger change commandeers the logger by default, avoiding unexpected logging to stdout and allows for the js log function to be switched out at runtime. I have replaced most of the workers with async functions, the only ones I haven't replaced are the ones for which async functions are not exposed to the c api. I've raised an issue for this https://github.com/apache/pulsar/issues/14452, but it should be noted that the library is still prone to segfaulting if the read workers execute after the client close call. This is my first time contributing to this library and I'm not particularly experienced with C++, but I have written node addons before. I have run the tests and checked that the linting and formatter have been run. If there are concerns around the style (such as the use of auto) I would welcome the feedback. -- 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
