search

confluentinc / confluent-kafka-javascript

Confluent's Apache Kafka JavaScript client
https://www.npmjs.com/package/@confluentinc/kafka-javascript
MIT License
50 stars 5 forks source link

fix: memory leak in incremental assign #35

Closed martijnimhoff closed 5 months ago

martijnimhoff commented 5 months ago

The incremental assign contained a memory leak. This happened because when unassigning, the wrong new pointers are destroyed (m_partitions).

Instead the partitions argument should be destroyed, because this is only used to find the right toppars in m_partitions. And the toppars in m_partitions which are unassigned also should be destroyed.

milindl commented 5 months ago

/sem-approve

martijnimhoff commented 5 months ago

I didn't push properly, so the comments weren't added, but that is done now!

milindl commented 5 months ago

@martijnimhoff it looks good to me. As this is our first external PR with code changes, there's a bunch of minor things I'm doing to get the CI to run on this once, and there should be a message from our CLA bot in a while on this PR for you. Thanks!

milindl commented 5 months ago

Here's the link to the CLA that should be signed: https://cla-assistant.io/confluentinc/confluent-kafka-javascript?pullRequest=35

cla-assistant[bot] commented 5 months ago

CLA assistant check
All committers have signed the CLA.

martijnimhoff commented 5 months ago

I've signed it. Should be fine now?

milindl commented 5 months ago

/sem-approve

milindl commented 5 months ago

Closing and reopening PR to try and trigger semaphore CI checks to run.

milindl commented 5 months ago

/sem-approve

milindl commented 5 months ago

Thanks @martijnimhoff for the fix!