Skip to content

Do not use SIGTERM #49352

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Do not use SIGTERM #49352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ec351d
Do not use SIGTERM
tmat Jun 11, 2025
16136cf
Fix
tmat Jun 11, 2025
dbcfb44
Kill
tmat Jun 12, 2025
3e7cf33
Fix
tmat Jun 12, 2025
6902c72
SKip
tmat Jun 12, 2025
eaab8f1
Fix
tmat Jun 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ private static void TerminateProcess(Process process, ProcessState state, IRepor
}
else
{
TerminateUnixProcess(state, reporter);
TerminateUnixProcess(process, state, reporter);
}

reporter.Verbose($"Process {state.ProcessId} killed.");
Expand Down Expand Up @@ -305,17 +305,21 @@ private static void TerminateWindowsProcess(Process process, ProcessState state,
process.Kill();
}

private static void TerminateUnixProcess(ProcessState state, IReporter reporter)
private static void TerminateUnixProcess(Process process, ProcessState state, IReporter reporter)
{
[DllImport("libc", SetLastError = true, EntryPoint = "kill")]
static extern int sys_kill(int pid, int sig);
// Signals no longer work on Linux https://github.com/dotnet/sdk/issues/49307

var result = sys_kill(state.ProcessId, state.ForceExit ? SIGKILL : SIGTERM);
if (result != 0)
{
var error = Marshal.GetLastPInvokeError();
reporter.Verbose($"Error while sending SIGTERM to process {state.ProcessId}: {Marshal.GetPInvokeErrorMessage(error)} (code {error}).");
}
//[DllImport("libc", SetLastError = true, EntryPoint = "kill")]
//static extern int sys_kill(int pid, int sig);

//var result = sys_kill(state.ProcessId, state.ForceExit ? SIGKILL : SIGTERM);
//if (result != 0)
//{
// var error = Marshal.GetLastPInvokeError();
// reporter.Verbose($"Error while sending SIGTERM to process {state.ProcessId}: {Marshal.GetPInvokeErrorMessage(error)} (code {error}).");
//}

process.Kill(entireProcessTree: true);
}
}
}
25 changes: 18 additions & 7 deletions test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ class AppUpdateHandler
}
}

[Theory(Skip = "https://github.com/dotnet/sdk/issues/45299")]
[Theory]
[CombinatorialData]
public async Task BlazorWasm(bool projectSpecifiesCapabilities)
{
Expand Down Expand Up @@ -502,7 +502,7 @@ public async Task BlazorWasm_MSBuildWarning()
await App.AssertWaitingForChanges();
}

[Fact(Skip = "https://github.com/dotnet/sdk/issues/49307")]
[Fact]
public async Task BlazorWasm_Restart()
{
var testAsset = TestAssets.CopyTestAsset("WatchBlazorWasm")
Expand Down Expand Up @@ -757,7 +757,7 @@ public static void PrintDirectoryName([CallerFilePathAttribute] string filePath
await App.AssertOutputLineStartsWith("> NewSubdir");
}

[Fact]
[Fact(Skip = "https://github.com/dotnet/sdk/issues/49307")]
public async Task Aspire()
{
var tfm = ToolsetInfo.CurrentTargetFramework;
Expand Down Expand Up @@ -817,9 +817,13 @@ public async Task Aspire()
}
else
{
App.AssertOutputContains($"dotnet watch ❌ [WatchAspire.ApiService ({tfm})] Exited");

// TODO: SIGTERM no longer works on Linux https://github.com/dotnet/sdk/issues/49307
// Unix process may return exit code = 128 + SIGTERM

// Exited with error code 143
App.AssertOutputContains($"[WatchAspire.ApiService ({tfm})] Exited");
// App.AssertOutputContains($"[WatchAspire.ApiService ({tfm})] Exited");
}

App.AssertOutputContains($"dotnet watch ⌚ Building {serviceProjectPath} ...");
Expand All @@ -834,7 +838,7 @@ public async Task Aspire()
await App.AssertOutputLineStartsWith($"dotnet watch ⌚ [WatchAspire.ApiService ({tfm})] Capabilities");

App.AssertOutputContains($"dotnet watch 🔨 Build succeeded: {serviceProjectPath}");
App.AssertOutputContains("dotnet watch 🔥 Project baselines updated.");
App.AssertOutputContains("dotnet watch 🔥 Projects rebuilt");
App.AssertOutputContains($"dotnet watch ⭐ Starting project: {serviceProjectPath}");

// Note: sending Ctrl+C via standard input is not the same as sending real Ctrl+C.
Expand All @@ -843,6 +847,8 @@ public async Task Aspire()

await App.AssertOutputLineStartsWith("dotnet watch 🛑 Shutdown requested. Press Ctrl+C again to force exit.");

// SIGTERM no longer works on Linux https://github.com/dotnet/sdk/issues/49307

// We don't have means to gracefully terminate process on Windows, see https://github.com/dotnet/runtime/issues/109432
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -851,10 +857,15 @@ public async Task Aspire()
}
else
{
// TODO: SIGTERM no longer works on Linux https://github.com/dotnet/sdk/issues/49307

await App.AssertOutputLineStartsWith($"dotnet watch ❌ [WatchAspire.ApiService ({tfm})] Exited");
await App.AssertOutputLineStartsWith($"dotnet watch ❌ [WatchAspire.AppHost ({tfm})] Exited");

// Unix process may return exit code = 128 + SIGTERM
// Exited with error code 143
await App.AssertOutputLine(line => line.Contains($"[WatchAspire.ApiService ({tfm})] Exited"));
await App.AssertOutputLine(line => line.Contains($"[WatchAspire.AppHost ({tfm})] Exited"));
// await App.AssertOutputLine(line => line.Contains($"[WatchAspire.ApiService ({tfm})] Exited"));
// await App.AssertOutputLine(line => line.Contains($"[WatchAspire.AppHost ({tfm})] Exited"));
}

await App.AssertOutputLineStartsWith("dotnet watch ⭐ Waiting for server to shutdown ...");
Expand Down
Loading