Bug 469971 - Helgrind reports possible data race which isn't
Summary: Helgrind reports possible data race which isn't
Status: RESOLVED INTENTIONAL
Alias: None
Product: valgrind
Classification: Developer tools
Component: helgrind (other bugs)
Version First Reported In: 3.18.1
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-19 02:07 UTC by h.teisseire
Modified: 2023-05-19 06:44 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description h.teisseire 2023-05-19 02:07:05 UTC
SUMMARY
***
Using `valgrind --tool=helgrind ./philo 5 300 100 50` a possible data race is reported that shouldn't possible.
Because at the end of the routine I call in a newly created thread I lock/unlock a mutex. And in my main thread I make a call to pthread_mutex_destroy().
***


STEPS TO REPRODUCE
1. Download my project source file from https://github.com/teisseire117/cursus42/tree/master/Philosophers/philo and compile the project using the command `make`
2. Launch the program with valgrind using the command `valgrind --tool=helgrind ./philo 5 300 100 50`
3. It will report the above-mentioned possible data race.

OBSERVED RESULT
Possible data race report from Helgrind

EXPECTED RESULT
No error reports were expected.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Linux hateisse-VB 5.19.0-41-generic #42~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Apr 18 17:40:00 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

ADDITIONAL INFORMATION
Before making a call to pthread_mutex_destroy() I ensure that the thread in which I try to lock/unlock will not be using that mutex because it would have made a final return which will exit the thread. So it shouldn't be possible for pthread_mutex_destroy() to have a data race because it's only called when no more lock/unlock of that mutex are possible to happen.
(It's always possible that I'm wrong but I'm reading it as open-minded as I can and I don't see the issue. If you find a real issue then I would be glad to have learned why !)

Thank you
Comment 1 h.teisseire 2023-05-19 02:10:47 UTC
I know it's always a hassle to read source code. Although I think I haven't made it very complicated. It should be quite straight-forward.
Comment 2 h.teisseire 2023-05-19 02:13:46 UTC
Oh and I also want to note that I'm learning programmation, so I know some of my code feels like it should have be done differently. For example I'm using pthread_detach when I could have used pthread_join which would have been more appropriate for what I try to do.
If I were to re-do the project I would have done it differently, however I really am here only for the Possible data race issue and why it happens in this specific case and if whether it's a "bug" or not.
Comment 3 Paul Floyd 2023-05-19 06:37:05 UTC
(In reply to h.teisseire from comment #2)
> Oh and I also want to note that I'm learning programmation, so I know some
> of my code feels like it should have be done differently. For example I'm
> using pthread_detach when I could have used pthread_join which would have
> been more appropriate for what I try to do.
> If I were to re-do the project I would have done it differently, however I
> really am here only for the Possible data race issue and why it happens in
> this specific case and if whether it's a "bug" or not.

 See point 6 here

https://valgrind.org/docs/manual/hg-manual.html#hg-manual.effective-use

Round up all finished threads using pthread_join. Avoid detaching threads: don't create threads in the detached state, and don't call pthread_detach on existing threads.

Using pthread_join to round up finished threads provides a clear synchronisation point that both Helgrind and programmers can see. If you don't call pthread_join on a thread, Helgrind has no way to know when it finishes, relative to any significant synchronisation points for other threads in the program. So it assumes that the thread lingers indefinitely and can potentially interfere indefinitely with the memory state of the program. It has every right to assume that -- after all, it might really be the case that, for scheduling reasons, the exiting thread did run very slowly in the last stages of its life.
Comment 4 h.teisseire 2023-05-19 06:41:22 UTC
(In reply to Paul Floyd from comment #3)
> (In reply to h.teisseire from comment #2)
> > Oh and I also want to note that I'm learning programmation, so I know some
> > of my code feels like it should have be done differently. For example I'm
> > using pthread_detach when I could have used pthread_join which would have
> > been more appropriate for what I try to do.
> > If I were to re-do the project I would have done it differently, however I
> > really am here only for the Possible data race issue and why it happens in
> > this specific case and if whether it's a "bug" or not.
> 
>  See point 6 here
> 
> https://valgrind.org/docs/manual/hg-manual.html#hg-manual.effective-use
> 
> Round up all finished threads using pthread_join. Avoid detaching threads:
> don't create threads in the detached state, and don't call pthread_detach on
> existing threads.
> 
> Using pthread_join to round up finished threads provides a clear
> synchronisation point that both Helgrind and programmers can see. If you
> don't call pthread_join on a thread, Helgrind has no way to know when it
> finishes, relative to any significant synchronisation points for other
> threads in the program. So it assumes that the thread lingers indefinitely
> and can potentially interfere indefinitely with the memory state of the
> program. It has every right to assume that -- after all, it might really be
> the case that, for scheduling reasons, the exiting thread did run very
> slowly in the last stages of its life.

Alright I understand, thank you for your quick answer :)