Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix usage of pthread_cond_timedwait #491

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

alexmohr
Copy link
Contributor

@alexmohr alexmohr commented May 25, 2023

pthread_cond_timedwait has to be called with a locked mutex using an unlocked mutex is undefined behaviour
although it works on many platforms

The program was tested solely for our own use cases, which might differ from yours.
Licensed under Mozilla Public License Version 2.0

Alexander Mohr, alexander.m.mohr@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH, imprint

pthread_cond_timedwait has to be called with a locked mutex
using an unlocked mutex is undefined behaviour
although it works on many platforms

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
alexmohr and others added 2 commits May 31, 2023 14:47
this commit makes sure that we do not wait
for up to 10s if we missed the signal
for the condition variable the first time

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
@@ -3775,7 +3775,9 @@ void dlt_user_housekeeperthread_function(void *ptr)

// signal dlt thread to be running
*dlt_housekeeper_running = true;
pthread_mutex_lock(&dlt_housekeeper_running_mutex);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my understading, the lock does not need to be hold while pthread_cond_signal() is called.
From posix specification:
"The pthread_cond_signal() or pthread_cond_broadcast() functions may be called by a thread whether or not it currently owns the mutex..."
Reference: https://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_cond_signal.html

I am fine with the rest of the changes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @alexmohr ,
i just fixed my finding by myself. If you give me a 👍 , I'll merge the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it! I did not find the time to do it myself :)

@michael-methner michael-methner merged commit d3d4bc3 into COVESA:master Aug 9, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants