-
Notifications
You must be signed in to change notification settings - Fork 11
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
Timeouts #86
Conversation
88be16c
to
11050ca
Compare
7faa03c
to
8ebe4eb
Compare
3c21e18
to
9f98d45
Compare
Benchmarks seem to indicate minor acceptable overhead due to optionally setting up the timeouts:
|
be42de5
to
379d83c
Compare
Fun.protect ~finally @@ fun () -> | ||
(match op ~timeoutf:0.1 x with | ||
| () -> assert false | ||
| exception Timeout.Timeout -> ()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used Alcotest.check_raises
here, but I saw the following error on CI
[exception] Stdlib.Queue.Empty
Raised at Stdlib__Domain.join in file "domain.ml", line 258, characters 16-24
Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15
Called from Alcotest_engine__Core.Make.protect_test.(fun) in file "src/alcotest-engine/core.ml", line 181, characters 17-23
Called from Alcotest_engine__Monad.Identity.catch in file "src/alcotest-engine/monad.ml", line 24, characters 31-35
which I assume comes from inside Alcotest, because Stdlib.Queue
is not used anywhere in kcas.
See issue in alcotest.
Based on a scheduler independent domain-local-timeout mechanism for setting timeouts, i.e. callback to be called after specified amount of time, this PR adds an optional
timeoutf
argument to specify a timeout in seconds for potentially blocking operations.