Skip to content

Conversation

@RalfJung
Copy link
Member

Currently, we check for expired timeouts after each step of execution. That seems excessive. This changes the scheduler to only check for timeouts when the active thread cannot continue running any more.

@vakaras does this sound right? pthread_cond_timedwait anyway already yields, of course, since it blocks on getting the signal (or the timeout).

@vakaras
Copy link
Contributor

vakaras commented Apr 11, 2021

@vakaras does this sound right? pthread_cond_timedwait anyway already yields, of course, since it blocks on getting the signal (or the timeout).

Yes, this sounds correct. However, I noticed that there seems to be no test for this. I would suggest adding it to this file:

libc::pthread_cond_timedwait(&mut cond as *mut _, &mut mutex as *mut _, &timeout),

@RalfJung
Copy link
Member Author

I added a test but it doesn't seem terribly meaningful -- in particular to really check the condition in the spec that is relevant here, we'd need some other thread to be already ready to give a signal so that there is a race between that signal and the timeout. I am not sure how to construct such a situation.

@vakaras
Copy link
Contributor

vakaras commented Apr 11, 2021

I think you cannot make a useful test without taking into account the scheduler. However, I think you could exploit the knowledge that the current scheduler never preempts the thread:

  1. Prepare the conditional variable.
  2. Spin the second thread, get it ready, and yield.
  3. On the first thread, call pthread_cond_timedwait.
  4. Signal on the second thread.
  5. Check that the first thread got ETIMEDOUT.

@RalfJung
Copy link
Member Author

Hm... I think I'll leave adding such a test to future work.^^ If you want to give it a try, feel free to. :)

@bors r+

@bors
Copy link
Contributor

bors commented Apr 11, 2021

📌 Commit 0674d43 has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Apr 11, 2021

⌛ Testing commit 0674d43 with merge e6ffc68...

@bors
Copy link
Contributor

bors commented Apr 11, 2021

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing e6ffc68 to master...

@bors bors merged commit e6ffc68 into rust-lang:master Apr 11, 2021
@RalfJung RalfJung deleted the less-timeout-checking branch April 11, 2021 16:17
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.

3 participants