Skip to content

Commit

Permalink
tests: Add a new Helgrind suppression
Browse files Browse the repository at this point in the history
Current versions of Helgrind complain that inside of `take_gil`:

    Thread #1: pthread_cond_{signal,broadcast}: dubious: associated lock is not held by any thread
       at 0x4851C1D: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
       by 0x4CA4E37: __pthread_cond_wait_common (pthread_cond_wait.c:516)
       by 0x4CA4E37: pthread_cond_timedwait@@GLIBC_2.3.2 (pthread_cond_wait.c:652)
       by 0x4855C72: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
       by 0x4A0A1D7: PyCOND_TIMEDWAIT (condvar.h:73)
       by 0x4A0A1D7: take_gil (ceval_gil.h:256)
       by 0x4A0A3B5: PyEval_RestoreThread (ceval.c:547)
       by 0x4AB66C8: acquire_timed (_threadmodule.c:103)
       by 0x4AB64DE: lock_PyThread_acquire_lock (_threadmodule.c:183)
       by 0x49D1C00: cfunction_call (methodobject.c:543)
       by 0x49AE39D: _PyObject_MakeTpCall (call.c:215)
       by 0x493F2FA: PyObject_Vectorcall (abstract.h:123)
       by 0x493F2FA: trace_call_function.cold (ceval.c:5864)
       by 0x4A116E9: call_function (ceval.c:5890)
       by 0x4A116E9: _PyEval_EvalFrameDefault (ceval.c:4198)
       by 0x4A0A413: _PyEval_EvalFrame (pycore_ceval.h:46)
       by 0x4A0A413: _PyEval_Vector (ceval.c:5067)

and that when stopping Memray's background thread:

    Thread #1: pthread_cond_{signal,broadcast}: dubious: associated lock is not held by any thread
       at 0x4851C1D: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
       by 0x8E04D58: memray::tracking_api::Tracker::BackgroundThread::stop() (tracking_api.cpp:736)
       by 0x8E0616B: memray::tracking_api::Tracker::~Tracker() (tracking_api.cpp:599)
       by 0x8E097A2: std::default_delete<memray::tracking_api::Tracker>::operator()(memray::tracking_api::Tracker*) const (unique_ptr.h:99)
       by 0x8E062B2: reset (unique_ptr.h:211)
       by 0x8E062B2: reset (unique_ptr.h:509)
       by 0x8E062B2: memray::tracking_api::Tracker::destroyTracker() (tracking_api.cpp:1080)
       by 0x8DA68C3: __pyx_pf_6memray_7_memray_7Tracker_4__exit__(__pyx_obj_6memray_7_memray_Tracker*, _object*, _object*, _object*) (_memray.cpp:23267)
       by 0x8DA73D5: __pyx_pw_6memray_7_memray_7Tracker_5__exit__(_object*, _object* const*, long, _object*) (_memray.cpp:23175)
       by 0x8D86488: __Pyx_CyFunction_Vectorcall_FASTCALL_KEYWORDS(_object*, _object* const*, unsigned long, _object*) (_memray.cpp:52036)
       by 0x49B0503: _PyObject_VectorcallTstate (abstract.h:114)
       by 0x49B0503: method_vectorcall (classobject.c:53)
       by 0x4A7FD91: _PyObject_VectorcallTstate (abstract.h:114)
       by 0x4A11962: call_function (ceval.c:5890)
       by 0x4A11962: _PyEval_EvalFrameDefault (ceval.c:4213)
       by 0x4A0A413: _PyEval_EvalFrame (pycore_ceval.h:46)
       by 0x4A0A413: _PyEval_Vector (ceval.c:5067)

pthreads don't actually require the lock to be held when signalling:

> The `pthread_cond_broadcast()` or `pthread_cond_signal()` functions
> may be called by a thread whether or not it currently owns the mutex
> that threads calling `pthread_cond_wait()` or
> `pthread_cond_timedwait()` have associated with the condition variable
> during their waits; however, if predictable scheduling behavior is
> required, then that mutex shall be locked by the thread calling
> `pthread_cond_broadcast()` or `pthread_cond_signal()`.

So this seems like a false positive, not a bug.

Signed-off-by: Matt Wozniski <[email protected]>
  • Loading branch information
godlygeek committed Feb 23, 2025
1 parent 55d77a0 commit 7a64eac
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions valgrind.supp
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,27 @@
...
}

# Helgrind false positives

# Helgrind warns if a condition variable is signalled while the lock is not
# held, but pthreads explicitly allow this case. Memray is doing this when
# stopping the background thread that collects RSS info, and CPython does this
# for the GIL.
{
<memray_signalling_cond_var_with_mutex_unlocked>
Helgrind:Dubious
...
fun:_ZN6memray12tracking_api7Tracker14destroyTrackerEv
...
}
{
<cpython_signalling_cond_var_with_mutex_unlocked>
Helgrind:Dubious
...
fun:take_gil
...
}

# libunwind

# `validate_mem` checks whether a memory region is both mapped and readable. It
Expand Down

0 comments on commit 7a64eac

Please sign in to comment.