#50523 Ticket 50490 objects and memory leaks
Closed by spichugi. Opened by lkrispen.
lkrispen/389-ds-base t50490  into  master

Download 50523.patch

Bug: There are severalmemory leaks for replication objects

Fix: This patch contains a couple of fixes:

- The balance of acquire and release for a replica object was incorrect,
but the object is allocated at startup or when a replica is added and
destroyed at shutdown. In between we know the replica exists and can be accessed directly
To ensure that no access was made until it is destroyed the shutdown order was
slightly modifed
- other objects like RUV or AGMT were also not always correctly balanced, this
is corrected
- in cl5_api where many types of objects are used, the variable names were changed
to bettr indicat to what an object refers
- some other leaks, eg in repl5_total_init or op_shared_add were fixed
- unused code has been removed

Reviewed by: ?

Very minor nitpick, but worth doing if (cond) { ... } IE add {} around the object_release.

Looks pretty reasonable, and I'm always a fan of a patch that removes more lines than it adds. I assume you have run this through asan + test suites?

Patch looks good to me, but I could not apply it on top of master to run further tests (compiler warnings, etc)

Patch looks good to me, but I could not apply it on top of master to run further tests (compiler warnings, etc)

I was able to get it applied, and it passed my build test. Ack

returning a replica, shouldn't it be named 'prot_get_replica'

Looks weird release the object gen_obj at the condition it contains data (gen), why not releasing it systematically if gen_obj!=NULL

Same comment as above

rebased onto 340f23990199698cb42b4a4e2686d3c404745800

Very minor nitpick, but worth doing if (cond) { ... } IE add {} around the object_release.
done

Patch looks good to me, but I could not apply it on top of master to run further tests (compiler warnings, etc)

the cleanallruv patch touched same functions, rebased

returning a replica, shouldn't it be named 'prot_get_replica'

done, and also changed replica_object to replica in the Protocol structs

Looks weird release the object gen_obj at the condition it contains data (gen), why not releasing it systematically if gen_obj!=NULL

I didn't change this code, just indentaio change because some if stament was removed, I think the csn generator object is the only one I didn't touch, and I don't want to do it :-)

Apart from this defensive argument, I think we always create objects with data, if there is an object without data there is something wrong. But this is for another cleanup round

Thanks Ludwig for the explanation. Indeed it looks object->data != NULL <=> object !=NULL.
ACK

Pull-Request has been merged by lkrispen

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3579

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata