repl5.h:
repl5_agmt.c: typedef struct repl5agmt { ... struct changecounter changecounters[MAX_NUM_OF_MASTERS]; / changes sent/skipped since server start up */ ... }
cl5_cache.c: struct clc_buffer { ... struct csn_seq_ctrl_block *buf_cscbs [MAX_NUM_OF_MASTERS]; }
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1031222
For me the changes are good. ack
Two minor remarks: in agmt_maxcsn_to_smod, the 'smod' is initialized before testing if there is agreements. If there is no agreement, it returns 1 so the 'smod->mod' may leak
in agmt_set_maxcsn, ra->maxcsn=NULL is suppressed. If for some reason we do not find maxcsn in the db ruv then ra->maxcsn will be unchanged (no call to slapi_ch_free_string(&ra->maxcsn)). Is that what you expect ?
Replying to [comment:7 tbordaz]:
For me the changes are good. ack Two minor remarks: in agmt_maxcsn_to_smod, the 'smod' is initialized before testing if there is agreements. If there is no agreement, it returns 1 so the 'smod->mod' may leak
I made that change so it wouldn't leak. It is only called from replica_write_ruv(), where it will always free the smod that was initialized in agmt_maxcsn_to_smod().
I will add a comment to the function stating that the smod always needs to be freed - even on error.
Yes, we don't want to remove the old maxcsn value unless there is a new one to replace it with. In fact this only called when the agmt is started, so usually it is NULL to being with. Unless the agmt has been stopped and restarted, etc. Either way, we still don't want to remove the old value unless its being replaced.
I would rather have the memory leak fixes be in a separate commit, in case we need to cherry pick separately.
revision 0001-Ticket-47587-hard-coded-limit-of-64-masters-in-agree.patch
Replying to [comment:9 rmeggins]:
New patch is attached. I'll do another commit for the ticket that actually introduced the memory leaks.
git merge ticket47587 Updating 3155bbb..bae797c Fast-forward ldap/servers/plugins/replication/cl5_clcache.c | 22 +++++++++++++++++----- ldap/servers/plugins/replication/repl5.h | 4 ++-- ldap/servers/plugins/replication/repl5_agmt.c | 18 ++++++++++++++---- 3 files changed, 33 insertions(+), 11 deletions(-)
git push origin master Counting objects: 17, done. Delta compression using up to 4 threads. Compressing objects: 100% (9/9), done. Writing objects: 100% (9/9), 1.50 KiB, done. Total 9 (delta 7), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git 3155bbb..bae797c master -> master
commit bae797c Author: Mark Reynolds mreynolds@redhat.com Date: Thu Dec 5 11:58:56 2013 -0500
1.3.2
b3b9c45..df5a061 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
1.3.1
f00321f..457cd16 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
1.3.0
b4e73f3..63e919f 389-ds-base-1.3.0 -> 389-ds-base-1.3.0
1.2.11
766e747..b5622fb 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
Metadata Update from @mreynolds: - Issue assigned to mreynolds - Issue set to the milestone: 1.2.11.26
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 issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/924
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.