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

dotnet fsi doesn't support Ctrl-C Interrupt - results in unhandled PlatformNotSupportedException #9397

Closed
eiriktsarpalis opened this issue Jun 4, 2020 · 14 comments
Labels
Area-FSI Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@eiriktsarpalis
Copy link
Member

Repro steps

In a dotnet fsi interactive session, when I interrupt a long-running operating (e.g. by typing Ctrl+C), an unhandled PlatformNotSupportedException will be thrown, killing the process:

$ dotnet fsi

Microsoft (R) F# Interactive version 11.0.0.0 for F# 5.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> System.Threading.Thread.Sleep(100_000) ;;

- Interrupt
Unhandled exception. System.PlatformNotSupportedException: Thread abort is not supported on this platform.
   at System.Threading.Thread.Abort()
   at FSharp.Compiler.Interactive.Shell.killerThread@1719.Invoke() in F:\workspace\_work\1\s\src\fsharp\fsi\fsi.fs:line 1727
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

Related information

Provide any related information (optional):

  • Operating system: Windows and Linux
  • .NET Runtime kind: .NET Core version 5.0.100-preview.6.20301.1
@TIHan TIHan added Area-FSI Bug and removed Bug labels Jun 5, 2020
@TIHan
Copy link
Contributor

TIHan commented Jun 5, 2020

Don't know if it's a bug on our end. It seems the API Thread.Abort is not supported on the platform for some reason?

@eiriktsarpalis
Copy link
Member Author

It seems the API Thread.Abort is not supported on the platform for some reason?

Correct, it's not support at all in .NET Core. I feel there's probably no good workaround to this, but perhaps it could be handled more gracefully.

@abelbraaksma
Copy link
Contributor

Is there another way than Thread.Abort to interrupt an operation? Otherwise, the Ctrl-C handler could be placed in a try/with and just break, but not print the exception?

I've noticed in VS in the FSI window that some exceptions make the prompt disappear (but you can still continue writing a new statement). Maybe that's related.

@cartermp
Copy link
Contributor

cartermp commented Jun 5, 2020

It seems like we're just not doing it right here. Sounds like the proper solution is cancellation: https://stackoverflow.com/questions/53465551/dotnet-core-equivalent-to-thread-abort

@abelbraaksma
Copy link
Contributor

Interesting, but that won't stop hanging threads, unless the cancelation token is queried regularly (if I understand that post correctly). In the same SO thread two forceful alternatives are also mentioned, they could be used as a last resort method of cancelation fails.

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Jun 5, 2020 via email

@TIHan TIHan added the Bug label Jun 5, 2020
@dsyme
Copy link
Contributor

dsyme commented Jun 23, 2020

I didn't know about this issue until now. I'm seriously surprised that .NET Core removed all support for Thread.Abort - it is needed in interruptible interactive execution scenarios.

There's not a whole lot we can do about this. We could try to cancel by setting a checkable flag and/or Async.DefaultCancellationToken but to be honest the cancellation feature just isn't going to be particularly useful in .NET Core F# Scripting at this stage.

@eiriktsarpalis
Copy link
Member Author

Here's a comment explaining the rationale behind deprecation. FWIW Thread.Abort() in of itself cannot guarantee cancellation in general, e.g. when running async code.

Perhaps then the best workaround here is to advance the repl state without attempting to cancel the worker thread. The user can then make the call whether to kill the session altogether, assuming the divergent thread significantly impacts the process health.

@dsyme
Copy link
Contributor

dsyme commented Aug 24, 2020

Perhaps then the best workaround here is to advance the repl state without attempting to cancel the worker thread. The user can then make the call whether to kill the session altogether, assuming the divergent thread significantly impacts the process health.

Yes, this seems sensible, though we'd need a new execution thread.

@KevinRansom We should consider this, rather than crashing.

@KevinRansom
Copy link
Member

I will spend some time thinking about this ... however, I think the proper job is to execute the script in a separate process, and then we can just do a process kill on it. I expect we make a ton of assumptions that the repl and the executed code are in the same process, but I think that a separate process is the way to go.

@dsyme
Copy link
Contributor

dsyme commented Aug 24, 2020

The general request for uncooperative cancellation in .NET Core for developer evaluation scenarios is now here: dotnet/runtime#41291.

@jkotas seems to think that it's plausible to recover this for non-production scenarios (that is, the abort is being issued by a developer/script-kiddy, and not routinely by code or users as part of production execution of apps). See dotnet/runtime#11369 (comment)

...the proper job is to execute the script in a separate process, and then we can just do a process kill on it. I expect we make a ton of assumptions that the repl and the executed code are in the same process, but I think that a separate process is the way to go.

The main rationale for Ctrl-C cancellation is that it doesn't require process restart (and specifically it doesn't require loss of established definitions and re-loading of massive data sets).

As @jkotas mentions using a separate process might be feasible if all work is submitted bia the debug API via FuncEval. Then Ctrl-C Abort is the same as FuncEval Abort. However then the VS debugger can't connect to that same process..... (among other potential issues like performance etc.)

@abelbraaksma
Copy link
Contributor

That's an interesting development and proposal and would certainly help here. I hope it will be considered :).

@cartermp cartermp added the Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. label Aug 31, 2020
@dsyme dsyme self-assigned this Sep 1, 2020
@cartermp cartermp added this to the Backlog milestone Sep 1, 2020
@goswinr
Copy link
Contributor

goswinr commented Sep 7, 2020

I will spend some time thinking about this ... however, I think the proper job is to execute the script in a separate process, and then we can just do a process kill on it. I expect we make a ton of assumptions that the repl and the executed code are in the same process, but I think that a separate process is the way to go.

@KevinRansom @dsyme My use case of F# scripting requires my FCS Evaluation sessions to be hosted in process. Would this change with the above proposal? Thread.Abort works well for me (on .NET Framework)

@jonsequitur jonsequitur changed the title Interrupting dotnet fsi results in unhandled PlatformNotSupportedException [External] Interrupting dotnet fsi results in unhandled PlatformNotSupportedException Sep 30, 2020
@jonsequitur jonsequitur removed the Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. label Sep 30, 2020
@jonsequitur jonsequitur assigned KevinRansom and unassigned dsyme Sep 30, 2020
@KevinRansom KevinRansom added the Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. label Sep 22, 2021
@KevinRansom KevinRansom removed their assignment Sep 22, 2021
@jonsequitur jonsequitur added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Jan 31, 2022
@dsyme dsyme changed the title [External] Interrupting dotnet fsi results in unhandled PlatformNotSupportedException dotnet fsi doesn't support Ctrl-C Interrupt - results in unhandled PlatformNotSupportedException Mar 31, 2022
@AntonLapounov
Copy link
Member

Fixed by #13428 and #13770.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-FSI Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Archived in project
Development

No branches or pull requests

10 participants