Bug 276818 - [libc] mtx_init memory leak
Summary: [libc] mtx_init memory leak
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: threads (show other bugs)
Version: 14.0-RELEASE
Hardware: Any Any
: --- Affects Many People
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-04 09:50 UTC by Hodong
Modified: 2024-02-15 08:43 UTC (History)
3 users (show)

See Also:
linimon: mfc-stable14?
linimon: mfc-stable13?


Attachments
fix a memory leak (894 bytes, patch)
2024-02-04 10:09 UTC, Hodong
no flags Details | Diff
fix a memory leak (822 bytes, patch)
2024-02-04 11:00 UTC, Hodong
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hodong 2024-02-04 09:50:50 UTC

    
Comment 1 Hodong 2024-02-04 09:53:53 UTC
Hello.
A memory leak occurs in mtx_init.

#include <threads.h>

int main ()
{
  mtx_t mtx;
  mtx_init (&mtx, mtx_plain);
  mtx_destroy (&mtx);
  return 0;
}

$ cc mtx.c -lstdthreads -o mtx

$ valgrind --leak-check=full ./mtx
==26748== Memcheck, a memory error detector
==26748== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==26748== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==26748== Command: ./mtx
==26748== 
==26748== 
==26748== HEAP SUMMARY:
==26748==     in use at exit: 1,748 bytes in 3 blocks
==26748==   total heap usage: 3 allocs, 0 frees, 1,748 bytes allocated
==26748== 
==26748== 20 bytes in 1 blocks are definitely lost in loss record 1 of 3
==26748==    at 0x484CDE4: malloc (vg_replace_malloc.c:446)
==26748==    by 0x4C791B2: pthread_mutexattr_init (in /lib/libthr.so.3)
==26748==    by 0x485EA9D: mtx_init (in /usr/lib/libstdthreads.so.0)
==26748==    by 0x2016FC: main (in /home/hodong/projects/snippets/mtx)
==26748== 
==26748== LEAK SUMMARY:
==26748==    definitely lost: 20 bytes in 1 blocks
==26748==    indirectly lost: 0 bytes in 0 blocks
==26748==      possibly lost: 0 bytes in 0 blocks
==26748==    still reachable: 1,728 bytes in 2 blocks
==26748==         suppressed: 0 bytes in 0 blocks
==26748== Reachable blocks (those to which a pointer was found) are not shown.
==26748== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==26748== 
==26748== For lists of detected and suppressed errors, rerun with: -s
==26748== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Comment 2 Hodong 2024-02-04 10:09:11 UTC
Created attachment 248171 [details]
fix a memory leak

pthread_mutexattr_destroy(&attr);

I added this.
This doesn't cause any memory leaks, but I don't know if it's the right way to do it or not.
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2024-02-04 10:15:36 UTC
If pthread_mutexattr_init() returned error, you should not call _destroy().
Comment 4 Hodong 2024-02-04 11:00:47 UTC
Created attachment 248174 [details]
fix a memory leak

I modified the patch file and uploaded it again.
Comment 5 Konstantin Belousov freebsd_committer freebsd_triage 2024-02-04 11:11:59 UTC
I propose the following modification, which avoids writing the call to
pthread_mutexattr_destroy() twice.  Are you fine with it?

diff --git a/lib/libstdthreads/mtx.c b/lib/libstdthreads/mtx.c
index 719ba6486e41..3027a4e48c8d 100644
--- a/lib/libstdthreads/mtx.c
+++ b/lib/libstdthreads/mtx.c
@@ -43,7 +43,7 @@ int
 mtx_init(mtx_t *mtx, int type)
 {
 	pthread_mutexattr_t attr;
-	int mt;
+	int mt, res;
 
 	switch (type) {
 	case mtx_plain:
@@ -60,11 +60,12 @@ mtx_init(mtx_t *mtx, int type)
 
 	if (pthread_mutexattr_init(&attr) != 0)
 		return (thrd_error);
-	if (pthread_mutexattr_settype(&attr, mt) != 0)
-		return (thrd_error);
-	if (pthread_mutex_init(mtx, &attr) != 0)
-		return (thrd_error);
-	return (thrd_success);
+	res = thrd_success;
+	if (pthread_mutexattr_settype(&attr, mt) != 0 ||
+	    pthread_mutex_init(mtx, &attr) != 0)
+		res = thrd_error;
+	pthread_mutexattr_destroy(&attr);
+	return (res);
 }
 
 int
Comment 6 Hodong 2024-02-04 11:42:45 UTC
(In reply to Konstantin Belousov from comment #5)

Good.
Comment 7 commit-hook freebsd_committer freebsd_triage 2024-02-04 11:52:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a03f768612ad98a886458197c531a0b92203bf84

commit a03f768612ad98a886458197c531a0b92203bf84
Author:     Hodong <hodong@nimfsoft.art>
AuthorDate: 2024-02-04 10:14:22 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2024-02-04 11:50:56 +0000

    libstdthreads: destroy mutexattr in mtx_init()

    PR:     276818
    MFC after:      1 week

 lib/libstdthreads/mtx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-02-09 00:39:20 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e4cfb5981d2bf54a9bf4c676f5177648644d7db6

commit e4cfb5981d2bf54a9bf4c676f5177648644d7db6
Author:     Hodong <hodong@nimfsoft.art>
AuthorDate: 2024-02-04 10:14:22 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2024-02-09 00:38:04 +0000

    libstdthreads: destroy mutexattr in mtx_init()

    PR:     276818

    (cherry picked from commit a03f768612ad98a886458197c531a0b92203bf84)

 lib/libstdthreads/mtx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2024-02-11 01:41:21 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=58ccdcc6ffc5d053b92ba0069f02cfbe9ff498b6

commit 58ccdcc6ffc5d053b92ba0069f02cfbe9ff498b6
Author:     Hodong <hodong@nimfsoft.art>
AuthorDate: 2024-02-04 10:14:22 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2024-02-11 01:40:28 +0000

    libstdthreads: destroy mutexattr in mtx_init()

    PR:     276818

    (cherry picked from commit a03f768612ad98a886458197c531a0b92203bf84)

 lib/libstdthreads/mtx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Comment 10 Paul Floyd 2024-02-15 08:43:20 UTC
(In reply to Hodong from comment #1)

Sorry for being a bit slow. I'm going to also want to look at 

==26748==    still reachable: 1,728 bytes in 2 blocks

Probably unrelated to the issue at hand here, but certainly a small example like this should have 0 leaks and 0 reachable. That'll probably mean adding more default suppressions to Valgrind. If necessary I'll open a new FreeBSD bugzilla item.