]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
lib/pthreadpool: protect jobs list from concurrent thread access
authorNoel Power <noel.power@suse.com>
Thu, 20 Nov 2025 09:00:35 +0000 (09:00 +0000)
committerStefan Metzmacher <metze@samba.org>
Sun, 18 Jan 2026 15:16:59 +0000 (15:16 +0000)
ThreadSanitizer identifies a data race with pool->jobs with concurrent
threads in test added in previous commit.

This commit protects the pool->jobs list

(trace and line numbers are from before glue fix in previous commit)

WARNING: ThreadSanitizer: data race (pid=13574)
  Write of size 8 at 0x7b6000020260 by thread T16:
    #0 pthreadpool_tevent_job_done ../../lib/pthreadpool/pthreadpool_tevent.c:405 (pthreadpool_tevent_unit_test_san+0x407080)
    #1 tevent_common_invoke_immediate_handler ../../lib/tevent/tevent_immediate.c:190 (libtevent-private-samba.so+0x8dbf)
    #2 pthreadpool_tevent_job_fn ../../lib/pthreadpool/pthreadpool_tevent.c:351 (pthreadpool_tevent_unit_test_san+0x406bc4)
    #3 pthreadpool_server ../../lib/pthreadpool/pthreadpool.c:655 (pthreadpool_tevent_unit_test_san+0x4043bd)
    #4 <null> <null> (libtsan.so.0+0x323cf)

  Previous write of size 8 at 0x7b6000020260 by thread T13:
    #0 pthreadpool_tevent_job_send ../../lib/pthreadpool/pthreadpool_tevent.c:342 (pthreadpool_tevent_unit_test_san+0x406a09)
    #1 do_nested_pthread_job ../../lib/pthreadpool/test_pthreadpool_tevent.c:463 (pthreadpool_tevent_unit_test_san+0x408932)
    #2 pthreadpool_tevent_job_fn ../../lib/pthreadpool/pthreadpool_tevent.c:351 (pthreadpool_tevent_unit_test_san+0x406bc4)
    #3 pthreadpool_server ../../lib/pthreadpool/pthreadpool.c:655 (pthreadpool_tevent_unit_test_san+0x4043bd)
    #4 <null> <null> (libtsan.so.0+0x323cf)

  Thread T16 (tid=13591, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x5ed75)
    #1 pthreadpool_create_thread ../../lib/pthreadpool/pthreadpool.c:711 (pthreadpool_tevent_unit_test_san+0x4045ac)
    #2 pthreadpool_add_job ../../lib/pthreadpool/pthreadpool.c:792 (pthreadpool_tevent_unit_test_san+0x40496f)
    #3 pthreadpool_tevent_job_send ../../lib/pthreadpool/pthreadpool_tevent.c:329 (pthreadpool_tevent_unit_test_san+0x4065e2)
    #4 test_pthreadpool_tevent_job_send_multiple_3 ../../lib/pthreadpool/test_pthreadpool_tevent.c:515 (pthreadpool_tevent_unit_test_san+0x408c25)
    #5 cmocka_run_one_test_or_fixture ../../third_party/cmocka/cmocka.c:2948 (libcmocka-private-samba.so+0x6f92)
    #6 __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 (libc.so.6+0x40e6b)

  Thread T13 (tid=13588, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x5ed75)
    #1 pthreadpool_create_thread ../../lib/pthreadpool/pthreadpool.c:711 (pthreadpool_tevent_unit_test_san+0x4045ac)
    #2 pthreadpool_add_job ../../lib/pthreadpool/pthreadpool.c:792 (pthreadpool_tevent_unit_test_san+0x40496f)
    #3 pthreadpool_tevent_job_send ../../lib/pthreadpool/pthreadpool_tevent.c:329 (pthreadpool_tevent_unit_test_san+0x4065e2)
    #4 test_pthreadpool_tevent_job_send_multiple_3 ../../lib/pthreadpool/test_pthreadpool_tevent.c:515 (pthreadpool_tevent_unit_test_san+0x408c25)
    #5 cmocka_run_one_test_or_fixture ../../third_party/cmocka/cmocka.c:2948 (libcmocka-private-samba.so+0x6f92)
    #6 __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 (libc.so.6+0x40e6b)

SUMMARY: ThreadSanitizer: data race ../../lib/pthreadpool/pthreadpool_tevent.c:405 in pthreadpool_tevent_job_done

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15958
Signed-off-by: Noel Power <noel.power@suse.com>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Sun Jan 18 15:16:59 UTC 2026 on atb-devel-224

lib/pthreadpool/pthreadpool_tevent.c

index 19cf2dfbc15239a4bec25ee51983ecaa2543de47..422a88c784abc7e7691c50d53791181687b1d010 100644 (file)
@@ -64,6 +64,11 @@ struct pthreadpool_tevent {
        pthread_mutex_t glue_mutex;
 
        struct pthreadpool_tevent_job_state *jobs;
+       /*
+        * Control access to the jobs
+        */
+       pthread_mutex_t jobs_mutex;
+
 };
 
 struct pthreadpool_tevent_job_state {
@@ -108,6 +113,13 @@ int pthreadpool_tevent_init(TALLOC_CTX *mem_ctx, unsigned max_threads,
                return ret;
        }
 
+       ret = pthread_mutex_init(&pool->jobs_mutex, NULL);
+       if (ret != 0) {
+               pthread_mutex_destroy(&pool->glue_mutex);
+               TALLOC_FREE(pool);
+               return ret;
+       }
+
        talloc_set_destructor(pool, pthreadpool_tevent_destructor);
 
        *presult = pool;
@@ -143,12 +155,21 @@ static int pthreadpool_tevent_destructor(struct pthreadpool_tevent *pool)
                return ret;
        }
 
+       ret = pthread_mutex_lock(&pool->jobs_mutex);
+       if (ret != 0 ) {
+               return ret;
+       }
        for (state = pool->jobs; state != NULL; state = next) {
                next = state->next;
                DLIST_REMOVE(pool->jobs, state);
                state->pool = NULL;
        }
 
+       ret = pthread_mutex_unlock(&pool->jobs_mutex);
+       if (ret != 0 ) {
+               return ret;
+       }
+
        ret = pthread_mutex_lock(&pool->glue_mutex);
        if (ret != 0) {
                return ret;
@@ -165,6 +186,7 @@ static int pthreadpool_tevent_destructor(struct pthreadpool_tevent *pool)
        }
 
        pthread_mutex_unlock(&pool->glue_mutex);
+       pthread_mutex_destroy(&pool->jobs_mutex);
        pthread_mutex_destroy(&pool->glue_mutex);
 
        pool->glue_list = NULL;
@@ -405,8 +427,17 @@ struct tevent_req *pthreadpool_tevent_job_send(
         */
        talloc_set_destructor(state, pthreadpool_tevent_job_state_destructor);
 
+       ret = pthread_mutex_lock(&pool->jobs_mutex);
+       if (tevent_req_error(req, ret)) {
+               return tevent_req_post(req, ev);
+       }
        DLIST_ADD_END(pool->jobs, state);
 
+       ret = pthread_mutex_unlock(&pool->jobs_mutex);
+       if (tevent_req_error(req, ret)) {
+               return tevent_req_post(req, ev);
+       }
+
        return req;
 }
 
@@ -479,8 +510,17 @@ static void pthreadpool_tevent_job_done(struct tevent_context *ctx,
                private_data, struct pthreadpool_tevent_job_state);
 
        if (state->pool != NULL) {
+               int ret;
+               ret = pthread_mutex_lock(&state->pool->jobs_mutex);
+               if (tevent_req_error(state->req, ret)) {
+                       return;
+               }
                DLIST_REMOVE(state->pool->jobs, state);
+               ret = pthread_mutex_unlock(&state->pool->jobs_mutex);
                state->pool = NULL;
+               if (tevent_req_error(state->req, ret)) {
+                       return;
+               }
        }
 
        if (state->req == NULL) {