FAKE does not kill child processes on exit #2407
Labels
Comments
|
Interestingly, I get (on windows): |
|
Really interesting... I also never understand this line Is |
|
The logic for this is here: Basically all processes are added to this list. As long as
Don't worry about it, when we detect some mono we initialize variables here: https:/fsharp/FAKE/blob/e7f52f93018182273a802c4c7c873e57d1d7bcad/src/app/Fake.Core.Process/Process.fs#L908-L930 |
|
I just tested CTRL+C if it works in that scenario as well. And indeed it does: Can you run Fake with the verbose flag ( |
|
Sure |
|
Ok I think I know what happens as I see something similar on Ubuntu (WSL): It seems like dotnet is spawning child-processes and then existing itself, basically escaping the fake logic as we do not handle recursively spawned processes, so I think this is a duplicate of #1427 |
|
One more strange thing, I modified build script to able test Target.create "Run" (fun _ ->
CreateProcess.fromRawCommandLine "dotnet" "fsi ./script.fsx"
|> (Proc.start >> ignore)
)
Target.create "Sleep" (fun _ ->
System.Threading.Thread.Sleep(7000)
)
Target.create "All" ignore
"Run"
==> "Sleep"
==> "All"And if I press |
|
Does it mean that at the moment |
|
I can add a more detailed error to see why Ctrl+C fails, but I guess the reason is the same, the process has exited already.
Yes as far as I can see it works as expected
No the only way is to kill processes including their child, but there is no cross platform way to do it. Maybe https:/aspnet/AspNetCore/blob/master/src/Shared/Process/ProcessExtensions.cs but I don't think that logic works when the parent has already exited. Note that this also means that let p = Process.Start("dotnet fsi ./script.fsx")
Thread.Sleep(100)
p.Kill()will not work either |
|
This maybe should be reported to the dotnet cli repository, or send a PR for recursive kill (as long as it works cross platform). But at the moment I don't see any actual issue. I'm not sure what your scenario is but if you use |
|
I know three use cases where I had this issue Use case 1: SwaggerProvider - test api serverI start server with test api and then I want to compile TP again this server and stop it somehow at the end. https:/fsprojects/SwaggerProvider/blob/dev/build.fsx#L94-L98 in .net core world on macOS its keep dev server running Use case 2: Local dev server for documentation
|
|
@sergey-tihon Don't get me wrong, I want to get this fixed. But I don't see any way to do it properly. That's why I suggested opening an issue on the SDK side. If you find a way to kill the processes via regular .NET APIs we can integrate it into FAKE. I'd even accept PInvoke or hacks as long as they do not only consider unix or windows (I expect a PR to work on both) |
matthid
added a commit
that referenced
this issue
Oct 19, 2019
|
Seems like I guess we would have to backport the implementation, but it looks really really ugly |
|
Or we update the runner to netcore 3 and call that API via reflection if it exists, this would be a partial solution but probably better than nothing. |
|
Here's a snippet of what we do in our mac/linux environment to combat this issue: let private pipeline filename args =
CreateProcess.fromRawCommand filename args
|> CreateProcess.withTimeout (System.TimeSpan.FromSeconds(1.))
|> CreateProcess.redirectOutput
let execProcAndReturnMessages filename args =
pipeline filename args
|> CreateProcess.ensureExitCode
|> Proc.run
let pgrep args = Process.execProcAndReturnMessages "pgrep" args
let kill args = Process.execProcAndReturnMessages "kill" args
let killTerm processId = kill ["-TERM"; string processId]
let rec getAllChildIdsUnix parentId =
let result = pgrep [sprintf "-P %d" parentId]
if
result.Messages()
|> Seq.isEmpty
then
[parentId]
|> Seq.ofList
else
parentId
:: (
result.Messages()
|> Seq.toList
// |> Seq.cache
|> Seq.choose(
fun text ->
match System.Int32.TryParse(text) with
| (true, v) -> Some v
| _ -> None )
|> Seq.collect (getAllChildIdsUnix)
|> Seq.toList)
|> Seq.ofList
let killChildrenAndProcess _timeout parentId =
getAllChildIdsUnix parentId
|> Seq.rev
|> Seq.iter (killTerm >> ignore)yes it relies on |
|
Maybe you can send a PR and somebody else can work on the windows side? This is actually pretty similar to what I linked above. |
|
Yeah it's a port of that dotnet core code from a long while ago |
|
Now we need the windows side and a PR ;) |
|
There has not been any activity in this issue for the last 3 months so it will be closed in 14 days if there is no activity. |
|
@matthid Can you please re-open the issue
Isn't FAKE currently a dotnet CLI tool published targetting netcoreapp3.1? Maybe it is possible to add this parameter now |
|
@Zaid-Ajaj I'm not a fan of keeping issues open just for the sake of it. While the cli tool have been updated we still use netstandard2.0 assemblies at runtime (because fake depends on the standard not on the concrete implementation). This is just the way things work at the moment. I feel like there isn't much value to invest in netcoreapp3 given that net5 shifts everything again. So the logical step would be to invest time to upgrade everything to net5 (which hasn't a stable release yet)... In any case just as I have written above I'd like to get this fixed and would likely accept a PR, given that the implementation is robust and cross platform. I hope that clears things up. |
|
That's fair enough, thanks for the quick answer :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
When I start child process (and do not wait for process result) it continue run when fake script finish.
Repro steps
Full repro is here - https:/sergey-tihon/fake-process-kill
./script.fsxthat we will run as child processExpected behavior
I expect that FAKE script kill child processes faster than process print all 15 numbers
If I understand @matthid correctly, FAKE should stop all child processes automatically
Actual behavior
Child process continue execution after FAKE script
Known workarounds
Not yet
But one of possible workaround may be to return
pidand write final target that kill child process bypidbut as far as I know there is no way to getpidfromProc.startRelated information
The text was updated successfully, but these errors were encountered: