Skip to content

[Issue 8311][pulsar-client-cpp] In C shim, free message ID pointer after hitting callback#8312

Closed
bschofield wants to merge 2 commits intoapache:masterfrom
bschofield:bschofield/c-memoryleak
Closed

[Issue 8311][pulsar-client-cpp] In C shim, free message ID pointer after hitting callback#8312
bschofield wants to merge 2 commits intoapache:masterfrom
bschofield:bschofield/c-memoryleak

Conversation

@bschofield
Copy link
Contributor

Fixes #8311

Motivation

The C/C++ glue code in c_Producer.cc:handle_producer_send() has a memory leak. Specifically, a pulsar_message_id_t is created but never freed. This leaks one object for every asynchronous message send, which adds up to a big problem if sending lots of messages.

Modifications

After calling the C callback, delete the memory allocated for the pulsar_message_id_t.

Verifying this change

I have manually verified that this change fixes the leak, using valgrind.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

  • Does this pull request introduce a new feature? no

@bschofield
Copy link
Contributor Author

bschofield commented Oct 20, 2020

@merlimat
Copy link
Contributor

@bschofield I agree that this is very unintuitive (and wrong from a perspective of ownership), but the problem was when this change was introduced.

Right now, there is code depending on this (bad) API. If we change that, it will segfault in these apps.

@bschofield
Copy link
Contributor Author

OK -- closing this PR in favor of a fix to the cgo interface instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in golang client (cgo interface)

2 participants