#50309 Ticket 50308 - Fix memory leaks for repeat binds and replication
Closed 3 years ago by spichugi. Opened 5 years ago by mreynolds.
mreynolds/389-ds-base ticket50308  into  master

@@ -589,6 +589,7 @@ 

          /* this is the first time we have a local change for the RID

           * we need to check what the consumer knows about it.

           */

+         csn_free(&buf->buf_cscbs[i]->consumer_maxcsn);

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.

          ruv_get_largest_csn_for_replica(

              buf->buf_consumer_ruv,

              buf->buf_cscbs[i]->rid,
@@ -832,6 +833,7 @@ 

          /* Send CSNs that are covered by the local RUV snapshot */

          if (csn_compare(buf->buf_current_csn, cscb->local_maxcsn) <= 0) {

              skip = 0;

+             csn_free(&cscb->consumer_maxcsn);

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.

              csn_dup_or_init_by_csn(&cscb->consumer_maxcsn, buf->buf_current_csn);

              break;

          }
@@ -842,11 +844,12 @@ 

           * are not sure if current_csn is the neighbor.

           */

          if (csn_time_difference(buf->buf_current_csn, cscb->local_maxcsn) == 0 &&

-             (csn_get_seqnum(buf->buf_current_csn) ==

-              csn_get_seqnum(cscb->local_maxcsn) + 1)) {

+             (csn_get_seqnum(buf->buf_current_csn) == csn_get_seqnum(cscb->local_maxcsn) + 1))

+         {

              csn_init_by_csn(cscb->local_maxcsn, buf->buf_current_csn);

              if (cscb->consumer_maxcsn) {

-                 csn_init_by_csn(cscb->consumer_maxcsn, buf->buf_current_csn);

+                 csn_free(&cscb->consumer_maxcsn);

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

+                 csn_dup_or_init_by_csn(cscb->consumer_maxcsn, buf->buf_current_csn);

              }

              skip = 0;

              break;

@@ -98,6 +98,8 @@ 

  int32_t

  slapi_td_set_dn(char *value)

  {

+     char *dn = pthread_getspecific(td_requestor_dn);

+     slapi_ch_free_string(&dn);

      if (pthread_setspecific(td_requestor_dn, value) != 0) {

          return PR_FAILURE;

      }

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

https://pagure.io/389-ds-base/issue/50308

Reviewed by: ?

Looks pretty reasonable to me, ack

rebased onto 6c2bb66

5 years ago

Pull-Request has been merged by mreynolds

5 years ago

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.

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.

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

@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-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/3368

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

3 years ago