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

US364 32.07.2 p13 Clarify spurious failure for try_acquire #359

Closed
wg21bot opened this issue Oct 25, 2019 · 5 comments · Fixed by cplusplus/draft#3477
Closed

US364 32.07.2 p13 Clarify spurious failure for try_acquire #359

wg21bot opened this issue Oct 25, 2019 · 5 comments · Fixed by cplusplus/draft#3477
Labels
Projects
Milestone

Comments

@wg21bot
Copy link
Collaborator

wg21bot commented Oct 25, 2019

The phrasing of the spurious failure case of semaphore try_acquire can confuse readers, who may parse it as being about blocking guarantees or a statement about QoI, rather than capturing various memory model subtleties as intended. Better would be to word this case similarly to mutex try_lock. The proposed change does so.

Proposed change:
Replace with "Effects: Attempts to atomically check if the counter is positive and decrement it by one if so, without blocking. If the counter is not decremented, there is no effect and try_acquire immediately returns. An implementation by fail to decrement the counter even if it is positive. [ Note: This spurious failure is normally uncommon, but allows interesting implementations based on a simple compare and exchange ([atomic]). -- end note] An implementation should ensure that try_acquire() does not consistently return false in the absence of contending semaphore operations.“

@wg21bot wg21bot added the LWG Library label Oct 25, 2019
@wg21bot wg21bot changed the title PL363 32.06.4 [thread.condition.condvarany] p13 Clarify spurious failure US364 32.07.2 p13 Clarify spurious failure for try_acquire Oct 25, 2019
@jensmaurer
Copy link
Member

jensmaurer commented Oct 26, 2019

Suggested wording from reflector discussion:

Effects: Performs an atomic read-modify-write operation ([intro.races]) which determines whether counter is greater than zero, and, if so decrements counter. The determination that counter is greater than zero may fail spuriously. [Note: This means that the semaphore might not be acquired even if it is available. – end note.] An implementation should ensure that determination does not fail in the absence of contending semaphore acquisitions.

@jensmaurer jensmaurer added the SG1 Concurrency label Oct 26, 2019
@BillyONeal
Copy link

After discussion with SG1 folks on Monday, here's a P/R that closely follows the NB comment but also fixes my 'what the atomic op is is not clear' concern. Note that only the first sentence is changed from the NB comment P/R:

Proposed change:
Replace with "Effects: Attempts to atomically decrement the counter if it is positive, without blocking. If the counter is not decremented, there is no effect and try_acquire immediately returns. An implementation by fail to decrement the counter even if it is positive. [ Note: This spurious failure is normally uncommon, but allows interesting implementations based on a simple compare and exchange ([atomic]). -- end note] An implementation should ensure that try_acquire() does not consistently return false in the absence of contending semaphore operations.“

@jwakely
Copy link
Member

jwakely commented Nov 5, 2019

N.B. "An implementation by may fail to decrement the counter even if it is positive."

@jensmaurer jensmaurer added this to Inbox in LWG Nov 7, 2019
@mclow
Copy link

mclow commented Nov 8, 2019

Resolved by the adoption of P1960

@jensmaurer
Copy link
Member

Accepted with modification. See paper P1960R0.

@jensmaurer jensmaurer added this to the CD C++20 milestone Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
LWG
Accepted
Development

Successfully merging a pull request may close this issue.

6 participants