Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR - Ticket 50308 - Fix memory leaks for repeat binds and replication #3368

Closed
389-ds-bot opened this issue Sep 13, 2020 · 8 comments
Closed
Labels
merged Migration flag - PR pr Migration flag - PR

Comments

@389-ds-bot
Copy link

389-ds-bot commented Sep 13, 2020

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50309


Description: Fixed two memory leaks:

- If a worker thread had multiple binds the "bind dn"
  thread data was leaked.
- Memory leak when processing changes in the changelog

Resolves: #3367

Reviewed by: ?

@389-ds-bot 389-ds-bot added merged Migration flag - PR pr Migration flag - PR labels Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-03-29 02:40:31

Looks pretty reasonable to me, ack

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2019-03-29 14:19:09

rebased onto 6c2bb66

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2019-03-29 14:19:55

Pull-Request has been merged by mreynolds389

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2019-03-29 14:49:06

Sorry to be late on that review
buf->buf_cscbs[i] was allocated (calloc) few lines above (clcache_new_cscb).
I think we had no leak at this point.

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2019-03-29 14:54:12

csn_dup_or_init_by_csn does not overwrite the consumer_maxcsn pointer but overwrite the content of it.
I do not see how it can create a leak.

@389-ds-bot
Copy link
Author

Comment from tbordaz (@tbordaz) at 2019-03-29 14:58:06

The same here. The content of consumer_maxcsn was overitten but not the pointer.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2019-03-29 15:31:56

@tbordaz, ASAN complains about these though :-/ I will double check if they are all needed, but some of them definitely were leaking. I have to fix a compiler warning anyway so I'll see what I can find...

@389-ds-bot
Copy link
Author

Patch
50309.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Migration flag - PR pr Migration flag - PR
Projects
None yet
Development

No branches or pull requests

1 participant