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

run command: should threads be able to keep themselves running after interrupt? #4894

Open
ceedubs opened this issue Apr 18, 2024 · 3 comments

Comments

@ceedubs
Copy link
Contributor

ceedubs commented Apr 18, 2024

I just noticed that when I did a ctrl-c on a run, the main thread stopped but a child thread continued to run. After trying a few variations, it seems like this happens if catchAll (or something similar based on tryEval) catches the thread killed exception. On the one hand, there's an argument for letting a thread catch the thread kill to clean up resources. But in the case of the thread being killed, it seems like it should have to reraise the failure or be shut down after a grace period or something.

Here is an example:

tmp = do
  ignore let fork do
    forever do
      res = catchAll do
        printLine (evalToText !Instant.now)
        sleep (milliseconds +100)
      match res with
        Right () -> ()
        Left e -> Debug.trace "got a failure" e
  sleep (seconds +100)
   ⍟ These new definitions are ok to `add`:
                             
      tmp : '{IO, Exception} ()

@unison/base/main> run tmp                      
Instant +1713469793 235734892
Instant +1713469793 367414925
Instant +1713469793 468249204
Instant +1713469793 569243246
Instant +1713469793 670188645
Instant +1713469793 771990297
Instant +1713469793 872949552
Instant +1713469793 973564480
Instant +1713469794 74432675 
Instant +1713469794 175106171
^C                           
Aborted.                     
trace: got a failure         
Failure                      
  (typeLink ThreadKilledFailure)
  "thread killed"            
  (Any ())
Instant +1713469794 252910045
@unison/base/main> Instant +1713469794 353668343
Instant +1713469794 454583056
Instant +1713469794 555611260
Instant +1713469794 656467061
Instant +1713469794 757736540
Instant +1713469794 858507061
Instant +1713469794 959351940
Instant +1713469795 60041167
Instant +1713469795 160757406
Instant +1713469795 261226859
Instant +1713469795 362192508
Instant +1713469795 463146384

@unison/base/main> Instant +1713469795 564318560
Instant +1713469795 665267908
Instant +1713469795 766755748
Instant +1713469795 867625487
Instant +1713469795 968571276
Instant +1713469796 70602390
Instant +1713469796 171477490
@SystemFw
Copy link
Contributor

it seems like it should have to reraise the failure

The correct protocol for interruption is to clean resources and rethrow, never swallow a ThreadKilled. However, when I did the latest change to some of the relevant code (to not log ThreadKilled) I decided to not change the current behaviour for now, to avoid muddying the waters until we understand what's going on

@ceedubs
Copy link
Contributor Author

ceedubs commented Apr 19, 2024

To be clear to those less familiar with the context, I think that @SystemFw is referring to changes that he made in the @unison/httpserver library, not to the Unison language itself.

@SystemFw in cases like that, where it is at the top level of a forked thread, I don't know if there is really a better option. fork only accepts {IO}, not Exception, and anything that bubbles up to the top of the fork is ultimately going to be ignored or just logged.

@SystemFw
Copy link
Contributor

I don't know if there is really a better option. fork only accepts {IO}, not Exception, and anything that bubbles up to the top of the fork is ultimately going to be ignored or just logged.

that's true, but if you see in the whole chain of calls from Tcp server and up there are quite a few places that call catchAll before getting to the top of the thread.

Also, just repeating what I said on Slack wrt worst case scenario: in the current Unison model on the Haskell runtime there is no way to guarantee the release of something, if you’re unlucky enough.
The primitive is :

catchAll : '{IO, Exception} a ->{IO} Either Failure a

and so if you have

res = catchAll do something
!finalizer

and you are unlucky enough, something will terminate naturally, but then interruption will hit in between res and !finalizer, and finalization won’t run. You cannot prevent this in any way in Unison only, it simply lacks the relevant primitive from Haskell (mask), whilst inheriting its super granular interruption semantics.

The situation is also complicated by what the scheme runtime does, and basically I have held back from designing proper semantics because my preliminary analysis is that supporting it on both the Haskell and the Scheme runtime is nigh impossible

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

No branches or pull requests

2 participants