Add macos-latest to CI's os matrix #228
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Nice. Huh, so somewhat expected the hangs start during cancellation tests (our most aggressive of the bunch). @guilledk would you mind also putting a 5m timeout on the actions run? |
|
Yeah looks like we're gonna need a mac peep to check these out. |
|
@goodboy Timeout done! |
goodboy
reviewed
Oct 23, 2021
.github/workflows/ci.yml
Outdated
| @@ -24,12 +24,12 @@ jobs: | |||
|
|
|||
| testing: | |||
| name: '${{ matrix.os }} Python ${{ matrix.python }} - ${{ matrix.spawn_backend }}' | |||
| timeout-minutes: 10 | |||
| timeout-minutes: 5 | |||
There was a problem hiding this comment.
You'll probably need to bump this again (i think) due to increases in the test set.
Every subactor in the tree now receives the socket (or whatever the mailbox type ends up being) during startup and can call the new `tractor._discovery.get_root()` function to get a portal to the current root actor in their tree. The main reason for adding this atm is to support nested child actors gaining access to the root's tty lock for debugging. Also, when a channel disconnects from a message loop, might as well kill all its rpc tasks.
This appears to demonstrate the same bug found in #156. It looks like cancelling a subactor with a child, while that child is running sync code, can result in the child never getting cancelled due to some strange condition where the internal nurseries aren't being torn down as expected when a `trio.Cancelled` is raised.
For reliable remote cancellation we need to "report" `trio.Cancelled`s (just like any other error) when exhausting a portal such that the caller can make decisions about cancelling the respective actor if need be. Resolves #156
Add `Actor._cancel_called` and `._cancel_complete` making it possible to determine whether the actor has started the cancellation sequence and whether that sequence has fully completed. This allows for blocking in internal machinery tasks as necessary. Also, always trigger the end of ongoing rpc tasks even if the last task errors; there's no guarantee the trio cancellation semantics will guarantee us a nice internal "state" without this.
This reverts commit 2b53c74.
This reverts commit 1710b64.
If the root calls `trio.Process.kill()` on immediate child proc teardown when the child is using pdb, we can get stdstreams clobbering that results in a pdb++ repl where the user can't see what's been typed. Not killing such children on cancellation / error seems to resolve this issue whilst still giving reliable termination. For now, code that special path until a time it becomes a problem for ensuring zombie reaps.
Finally this makes a cancelled root actor nursery not clobber child tasks which request and lock the root's tty for the debugger repl. Using an edge triggered event which is set after all fifo-lock-queued tasks are complete, we can be sure that no lingering child tasks are going to get interrupted during pdb use and tty lock acquisition. Further, even if new tasks do queue up to get the lock, the root will incrementally send cancel msgs to each sub-actor only once the tty is not locked by a (set of) child request task(s). Add shielding around all the critical sections where the child attempts to allocate the lock from the root such that it won't be disrupted from cancel messages from the root after the acquire lock transaction has started.
We may get multiple re-entries to debugger by `bp_forever` sub-actor now since the root will incrementally try to cancel it only when the tty lock is not held.
|
Woah I wrecked this PR, let me close it and re-do it |
|
@guilledk lol can we get a new version? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This simply adds
macos-latestto the testing envoirments.There seems to be a few different failures as well as hangs using the
triobackend.You can see the actions run on my fork: https://github.com/guilledk/tractor/actions/runs/1094209479