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

rw_mutex condition_variable deadlock #352

Open
BrettRToomey opened this issue Nov 2, 2024 · 0 comments
Open

rw_mutex condition_variable deadlock #352

BrettRToomey opened this issue Nov 2, 2024 · 0 comments

Comments

@BrettRToomey
Copy link

Hello, I understand that Linux is not currently supported, but I just wanted to point this out and ask if there's any way I could help.

// TODO(rjf): because pthread does not supply cv/rw natively, I had to hack
// this together, but this would probably just be a lot better if we just
// implemented the primitives ourselves with e.g. futexes
//
if(os_handle_match(cv, os_handle_zero())) { return 0; }
if(os_handle_match(mutex_rw, os_handle_zero())) { return 0; }
OS_LNX_Entity *cv_entity = (OS_LNX_Entity *)cv.u64[0];
OS_LNX_Entity *rw_mutex_entity = (OS_LNX_Entity *)mutex_rw.u64[0];
struct timespec endt_timespec;
endt_timespec.tv_sec = endt_us/Million(1);
endt_timespec.tv_nsec = Thousand(1) * (endt_us - (endt_us/Million(1))*Million(1));
B32 result = 0;
for(;;)
{
pthread_mutex_lock(&cv_entity->cv.rwlock_mutex_handle);
int wait_result = pthread_cond_timedwait(&cv_entity->cv.cond_handle, &cv_entity->cv.rwlock_mutex_handle, &endt_timespec);

On line 958, there's a wait on the cv's rw-workaround-mutex, but the thread still holds the reader lock. If this reader is waiting on a writer to publish results, like in the task system, the writer won't ever be able to publish.

Unless I'm mistaken, a way around this would be to unlock the reader lock after taking the rw-workaround-mutex. Then I think the rw-workaround-mutex would also have to be locked before signalling and broadcasting.

I see in your comment above that you would like to replace this with a futex and I was wondering if this was something I could contribute? Were you planning on implementing a condition variable or was there something smarter/simpler than that?

On a personal project I have ported your OS layer and Render layer to macOS and Metal hoping that if/when you look into macOS I can seed this project with an initial implementation. I understand that right now this is just noise and I have no expectation that you will want it merged in, but if you're interested please let me know and I can publish my efforts.

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

No branches or pull requests

1 participant