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

[Bug] temporal-sdk-dotnet process doesn't close if test is stopped during debug on Visual Studio 2022 #88

Open
ltlombardi opened this issue Dec 11, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@ltlombardi
Copy link

ltlombardi commented Dec 11, 2024

What are you really trying to do?

Debugging unit test.

Describe the bug

During debug, If I stop before debug finishes on Visual Studio, temporal-sdk-dotnet-1.3.1.0.exe never closes / stops running.
In the video I show normal operation, that temporal-sdk-dotnet process starts and closes, if test finished normally.
On the second part of the video, I do it again, but stop the test execution before it finishes and temporal-sdk-dotnet never goes away.
If I run another test, It creates a new instance and it starts to acumulate multiple orphaned instances.
image

20241211_150907.mp4

Minimal Reproduction

Demo project to run
TemporalDemo - Copy.zip

Environment/Versions

  • OS and processor: Windows 11, x86
  • Temporal Version: temporal-sdk-dotnet-1.3.1.0
  • Are you using Docker or Kubernetes or building Temporal from source? None. Unit tests doesn't use those.
@ltlombardi ltlombardi added the bug Something isn't working label Dec 11, 2024
@cretz
Copy link
Member

cretz commented Dec 11, 2024

If I stop before debug finishes on Visual Studio

StartLocalAsync returns an IAsyncDisposable. Does stopping the process in this manner properly dispose objects? Or does every async disposable not get disposed when stopping this way? This SO post suggests that killing the process in this manner will not properly dispose things which includes not properly stopping the external process we start.

@ltlombardi
Copy link
Author

ltlombardi commented Dec 12, 2024

Well, considering the SO post you provided, it seems the problem is with the way Visual Studio and the Xunit test explorer "plugin" work...
ChatGPT suggested doing a clean up during the start of the unit test, so that any left overs from a previous stopped run are cleaned. It is something that can reduce the problem, not fix it. Something like this:

public class UnitTest1
{
    public UnitTest1()
    {
        ProcessManager.KillMatchingProcesses();
    }

    public static class ProcessManager
    {
        private const string ProcessNamePrefix = "temporal-sdk-dotnet";

        public static void KillMatchingProcesses()
        {
            foreach (var process in Process.GetProcesses())
            {
                try
                {
                    if (process.ProcessName.StartsWith(ProcessNamePrefix, StringComparison.OrdinalIgnoreCase))
                    {
                        process.Kill();
                        process.WaitForExit(); // Ensure it's fully terminated
                        Console.WriteLine($"Killed process: {process.ProcessName} (ID: {process.Id})");
                    }
                }
                catch (Exception ex)
                {
                    Console.WriteLine($"Failed to terminate process {process.Id}: {ex.Message}");
                }
            }
        }
    }
}

@cretz
Copy link
Member

cretz commented Dec 12, 2024

Yes, you can add this to clean up if you do not properly dispose. This is the same for other non-sub/child processes started in an app that is terminated non-gracefully. Note you may affect other running tests in other processes that have started their own test servers though. I do not believe we would add this in the SDK though, rather we strongly suggest not killing processes in a non-graceful manner. For some users, they may intentionally start a detached test workflow environment from .NET code. it is the user's choice to not gracefully dispose the workflow environment, and some may choose to do so intentionally.

@ltlombardi
Copy link
Author

It is something can happens without intent.

  1. I start a unit test, with debug, to investigate why it was failing.
  2. I find the problem.
  3. I stop, to fix it, since there is not reason to let it finish.

I never though that it was better to let it finish, to avoid this kind of problems..

Thank you for the quick reply 🥇 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants