[fix][client] prevent garbage collection of token-supplier

[klaatu01]

When using the new async token‐supplier API (() => Promise<string>) introduced in #395, the JS Authentication object and its backing ThreadSafeFunction can be garbage‐collected if no live JS references remain. After the initial token expires, Pulsar’s broker will send a new auth challenge and the native code crashes when it tries to invoke a finalized TSFN.

By persisting a reference to the authentication object in the client it prevents the V8 garbage collector from dropping the token-supplier.

[klaatu01]

@RobertIndie @BewareMyPower
Noticed that the tests failed because open handles prevented jest from exiting, I am assuming I need to make alterations to the close function to make sure we deference the authRef that the client is holding?

Sorry I haven't used C++ much before, am I right to assume that's what I need to do?

[BewareMyPower]

Yeah, compared to a successful workflow before (https://github.com/apache/pulsar-client-node/actions/runs/13716725396/job/38363013887):

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  Pulsar Logging

      54 |
      55 |   static setLogHandler(params) {
    > 56 |     PulsarBinding.Client.setLogHandler(params);
         |                          ^
      57 |   }
      58 |
      59 |   static genCertFile() {

      at Function.setLogHandler (src/Client.js:56:26)
      at Object.<anonymous> (tests/end_to_end.test.js:402:21)


> pulsar-client@1.14.0-rc.0 docs
> typedoc

It seems that there are two more open handles that keep Jest from exiting:

Token Supplier Callback

      22 | class AuthenticationToken {
      23 |   constructor(params) {
    > 24 |     this.binding = new PulsarBinding.Authentication('token', params);
         |                    ^
      25 |   }
      26 | }
      27 |

      at new AuthenticationToken (src/AuthenticationToken.js:24:20)
      at Object.<anonymous> (tests/end_to_end.test.js:1290:20)


  ●  Token Supplier Callback

      22 | class AuthenticationToken {
      23 |   constructor(params) {
    > 24 |     this.binding = new PulsarBinding.Authentication('token', params);
         |                    ^
      25 |   }
      26 | }
      27 |

      at new AuthenticationToken (src/AuthenticationToken.js:24:20)
      at Object.<anonymous> (tests/end_to_end.test.js:1312:20)

[BewareMyPower]

SuppressDestruct() suppresses the destruction of authRef_, which is held by the Client instance. Could you try removing this call?

This method is used here:

pulsar-client-node/src/Client.cc

Line 88 in b79ead0

constructor.SuppressDestruct();

However, constructor is a static variable, whose lifetime is the same with the whole program. But here authRef_'s lifetime is the same with the Client. Suppressing the destruction of authRef_ could cause memory leak.

[BewareMyPower]

I didn't have time looking deeper for now. But I assume that when the Client destructs, the authRef_ will be destroyed as well unless you call SuppressDestruct().

The original issue happens just because auth is a local variable that is referenced by the client config's authentication field. However, since it's now a field of Client, whose lifetime should be longer than its config. We can now prevent the GC of auth.

[BewareMyPower]

/cc @RobertIndie Please help double confirm if there is anything wrong with my analysis above

[klaatu01]

If that's the case then perhaps removing that for the logger object will also stop that other open handle.

I am away this weekend, but will give this a go when I'm back.

[RobertIndie]

Thanks for your analysis. I think it's better to add a test to verify it.

[klaatu01]

Sorry I am on mobile just seen that you have done it and tests passed!

[RobertIndie]

Could you help add a test to verify this change?