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

CP-52039: Drop Semaphore from Xapi_stdext_threads #6094

Merged

Conversation

contificate
Copy link
Contributor

Prior to version 4.12, OCaml's standard threads library (systhreads) had no builtin concept of a semaphore, so one was implemented in Xapi_stdext_threads.

We replace all usages of this with Semaphore.Counting from the standard library and remove the implementation from Xapi_stdext_threads.

Technically, the interface provided by the previous semaphore is more general: it permits arbitrary adjustments to the semaphore's counter, allowing for a "weighted" style of locking. However, this is only used in one place (with a weight value of 1, which is the same decrement/increment value as normal).


All green on BVT+BST (207043) apart from 2 known issues. The number of users of the semaphore module in the codebase is quite low, which is why I think it's fine to drop the implementation entirely.

Prior to version 4.12, OCaml's standard threads library (systhreads) had
no builtin concept of a semaphore, so one was implemented in
Xapi_stdext_threads.

We replace all usages of this with Semaphore.Counting from the standard
library and remove the implementation from Xapi_stdext_threads.

Technically, the interface provided by the previous semaphore is more
general: it permits arbitrary adjustments to the semaphore's counter,
allowing for a "weighted" style of locking. However, this is only used
in one place (with a weight value of 1, which is the same
decrement/increment value as normal).

Signed-off-by: Colin James <[email protected]>
@contificate contificate added this pull request to the merge queue Oct 30, 2024
Merged via the queue into xapi-project:master with commit 864ecdd Oct 30, 2024
15 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.

3 participants