Added an optional timeout to the Mailbox.take method #27
Conversation
…nderlying Mutex.runLocked and ConditionalVariable.wait methods.
Fixed a lint for Timeout Test
|
I've released a temp package to pub.dev if anyone needs this functionality urgently |
| calloc.free(mailbox.ref.buffer); | ||
| calloc.free(mailbox); | ||
| static final finalizer = Finalizer((mailbox) { | ||
| final p = mailbox! as Pointer<_MailboxRepr>; |
There was a problem hiding this comment.
because I dev using strong type lint options and the linter was complaining, so I fixed it.
There was a problem hiding this comment.
What was it complaining about? The change makes the code less typed: finalizer had static type Finalizer<Pointer<_MailboxRepr>> after your change this type is lost and you are resorting to runtime casting for some reason.
Please revert this change.
There was a problem hiding this comment.
This is still not resolved
… the calculations for remaining time when the wait wakes up for sperious reasons. Change the while termination in takeTimed to check for _stateEmpty rather than !_stateFull in anticipation of the mailbox supporting multiple messages rather than jus the single message it currently supports.
…microsecond resolution for timeouts.
|
changes as requested and merged with main. |
|
I've just had to raise an issue with the dart team as ffi seems to be crashing with the latest merges from main. So perhaps best no to merge this pr until we see a response on the noted issue. |
| unawaited( | ||
| spawnLockedMutex(mutex.asSendable, const Duration(seconds: 10))); | ||
|
|
||
| /// give the isoalte a chance to start. |
There was a problem hiding this comment.
- Typo:
isolate. - do not use doc comments inside functions.
- Please capitalize sentences in comments and end them with a
..
There was a problem hiding this comment.
This is still not resolved?
…to visually parser.
…e logging messages. restored static type to finalizer. restored all of hte unit tests now that we have identified the cause of the crash.
…ynamic link of windows dll which can result in other windows system calls between when dart call GetLastError and when it is actually called in the windows subsystem.
|
I've removed the check for a closed mailbox from Mailbox.take() with a timeout to side set the FFI crash. Do we wait for an fix to ffi before we release? |
| final remainingTime = _remainingTime(timeout, start); | ||
| _condVar.wait(_mutex, timeout: remainingTime); | ||
| } | ||
| // if (_mailbox.ref.state == _stateClosed) { |
|
Tests are not passing and there are still unresolved comments. |
Added so a user can determine what a call to Mailbox.put failed (e.g. the box was full or closed).
|
Hi - thanks for the PR. FYI that the source-of-truth for this package has moved to our dart-lang/labs monorepo; https://github.com/dart-lang/labs/tree/main/pkgs/native_synchronization. I'll close this PR, but feel free to re-create it against the new package location if this PR is still active. |
|
The PR was stalled mostly because I was distracted - I will take a landing this code in the monorepo when I get a chance. |
This required changes to the underlying Mutex.runLocked and ConditionalVariable.wait methods.
The changes are backwards compatible simply adding an optional 'timeout' argument to each of the noted methods.
The changes provide greater flexibility in how the methods are used and will also aid in debugging dead locks (which is what prompted me to do the work).