#50984 Memory leaks in disk monitoring
Closed: wontfix by spichugi. Opened by spichugi.

Issue Description

Memory leaks are reported by the disk monitoring test suite:

9128ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
0 0x7fd43c0b2fb8 in __interceptor_realloc (/lib64/libasan.so.5+0xeffb8)
1 0x7fd43bb2d6ff in slapi_ch_realloc (/usr/lib64/dirsrv/libslapd.so.0+0xdb6ff)
2 0x7fd43bb2b6cf in slapi_ch_array_add_ext (/usr/lib64/dirsrv/libslapd.so.0+0xd96cf)
3 0x55e9f5e75420 in disk_mon_add_dir ldap/servers/slapd/daemon.c:258
4 0x55e9f5e7563d in disk_mon_get_dirs ldap/servers/slapd/daemon.c:285
5 0x55e9f5e760c9 in disk_monitoring_thread ldap/servers/slapd/daemon.c:453
6 0x7fd43941a567 (/lib64/libnspr4.so+0x2b567)

Indirect leak of 34 byte(s) in 1 object(s) allocated from:
0 0x7fd43bffed70 in strdup (/lib64/libasan.so.5+0x3bd70)
1 0x7fd43bb2d8fd in slapi_ch_strdup (/usr/lib64/dirsrv/libslapd.so.0+0xdb8fd)
2 0x55e9f5e75229 in disk_mon_get_mount_point ldap/servers/slapd/daemon.c:224
3 0x55e9f5e753a6 in disk_mon_add_dir ldap/servers/slapd/daemon.c:251
4 0x55e9f5e7563d in disk_mon_get_dirs ldap/servers/slapd/daemon.c:285
5 0x55e9f5e760c9 in disk_monitoring_thread ldap/servers/slapd/daemon.c:453
6 0x7fd43941a567 (/lib64/libnspr4.so+0x2b567)
Indirect leak of 2 byte(s) in 1 object(s) allocated from:
0 0x7fd43bffed70 in strdup (/lib64/libasan.so.5+0x3bd70)
1 0x7fd43bb2d8fd in slapi_ch_strdup (/usr/lib64/dirsrv/libslapd.so.0+0xdb8fd)
2 0x55e9f5e75229 in disk_mon_get_mount_point ldap/servers/slapd/daemon.c:224
3 0x55e9f5e753a6 in disk_mon_add_dir ldap/servers/slapd/daemon.c:251
4 0x55e9f5e755d0 in disk_mon_get_dirs ldap/servers/slapd/daemon.c:277
5 0x55e9f5e760c9 in disk_monitoring_thread ldap/servers/slapd/daemon.c:453
6 0x7fd43941a567 (/lib64/libnspr4.so+0x2b567)
SUMMARY: AddressSanitizer: 60 byte(s) leaked in 3 allocation(s).

Version-Release number of selected component (if applicable):
389-ds-base-1.4.2.4-6.module+el8.2.0+5509+885f7879


Metadata Update from @spichugi:
- Issue assigned to spichugi

Metadata Update from @mreynolds:
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Issue priority set to: major
- Issue set to the milestone: 1.4.1

https://pagure.io/389-ds-base/pull-request/50985

Metadata Update from @spichugi:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Main commit:
335b6de10..8ecada010 master -> master

One-line rule fix:
8ecada010..a171670d8 master -> master

Combined cherry-pick:
80f6dbdce..324b5b1d0 389-ds-base-1.4.1 -> 389-ds-base-1.4.1
73dd876e1..e56a2fad7 389-ds-base-1.4.2 -> 389-ds-base-1.4.2

Metadata Update from @spichugi:
- Issue status updated to: Open (was: Closed)

https://pagure.io/389-ds-base/pull-request/51148

7e1d80f63..327147cf7 master -> origin/master
384d03cc5..46b51b806 389-ds-base-1.4.2 -> 389-ds-base-1.4.2
00c476078..a681b444e 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

Metadata Update from @spichugi:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Hey @spichugi I'm seeing an error now in static checks at suse from this. It's on line 902 in main.c, it looks like you have:

char *disk_mon_check_diskspace(char **dirs, uint64_t threshold, uint64_t *disk_space);

Not threshold is a uint64_t, but you pass it a int64_t:

 897         int64_t threshold = 0;
...
 899         threshold = config_get_disk_threshold();
...
 902         dirstr = disk_mon_check_diskspace(dirs, threshold, &disk_space);

What do you think the right fix is here? Shourd disk_mon_check take a int64_t instead?

Hey @spichugi I'm seeing an error now in static checks at suse from this. It's on line 902 in main.c, it looks like you have:
char disk_mon_check_diskspace(char dirs, uint64_t threshold, uint64_t disk_space);

Not threshold is a uint64_t, but you pass it a int64_t:
897 int64_t threshold = 0;
...
899 threshold = config_get_disk_threshold();
...
902 dirstr = disk_mon_check_diskspace(dirs, threshold, &disk_space);

What do you think the right fix is here? Shourd disk_mon_check take a int64_t instead?

Oh, good catch!
Looks like a typo. I see no reason why threshold should be unsigned as we allow by the docs maximum 2^63 - 1 anyway.

https://pagure.io/389-ds-base/pull-request/51206

c0688a064..654d0ff26 master -> origin/master
82796f172..668475ae4 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

@mreynolds as per discussion in #51206 - Is it okay if we cherry-pick the static checks fix commit to 1.4.2?

@mreynolds as per discussion in #51206 - Is it okay if we cherry-pick the static checks fix commit to 1.4.2?

Yes please, and the proto-slap.h fix

92afa2ea7..6023396b7 389-ds-base-1.4.2 -> 389-ds-base-1.4.2

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/4037

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: fixed)

Metadata