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

Disposing ZeebeClient causes crashing in async code #105

Closed
0x15e opened this issue Apr 2, 2020 · 7 comments
Closed

Disposing ZeebeClient causes crashing in async code #105

0x15e opened this issue Apr 2, 2020 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@0x15e
Copy link

0x15e commented Apr 2, 2020

Describe the bug
The blocking call to channelToGateway.ShutdownAsync().Wait() causes the application to terminate (exit code 6) at the next async call when used within an async context.

To Reproduce
Create a C# console application and reference zb-client 0.15.0 (this can also be reproduced using the code in master). Add the following code to the Program.cs file:

using System;
using System.Threading.Tasks;

using Zeebe.Client;

namespace DisposeCrashRepro
{
    class Program
    {
        static async Task Main(string[] args)
        {
            Console.WriteLine("Building client");
            
            var zeebeClient = ZeebeClient.Builder()
                .UseGatewayAddress("localhost:26500")
                .UsePlainText()
                .Build();
            
            Console.WriteLine("Creating workflow instance");
            try
            {
                // The "Test__Crash" process is a simple workflow that has one service task that fails with
                // an error event (not a job fail).

                var result = await zeebeClient
                    .NewCreateWorkflowInstanceCommand()
                    .BpmnProcessId("Test__Crash")
                    .LatestVersion()
                    .Variables("{}")
                    .WithResult()
                    .Send();
            }
            catch (Exception ex)
            {
                Console.WriteLine($"Caught exception {ex.Message}");
            }
            
            try
            {
                Console.WriteLine("Disposing client");
                zeebeClient.Dispose();
                
                Console.WriteLine("Awaiting Task.Delay, which will crash the application");
                await Task.Delay(TimeSpan.FromSeconds(10));
            }
            catch (Exception)
            {
                Console.WriteLine("This line will never be executed because the program terminates");
            }
        }
    }
}

Expected behavior
Disposing the Zeebe client should not crash the application.

Enviroment (please complete the following information):

  • OS: Mac OS X 10.14.6 (18G3020)
  • OS: Docker image mcr.microsoft.com/dotnet/core/aspnet:3.1.201-buster
  • zb-client Version: 0.15.0

Additional context
I can provide the complete core dump that goes with the crash if you think that would be helpful.

With some testing, I found that this can be solved by retargeting zb-client to
netstandard2.1 and implementing IAsyncDisposable on ZeebeClient (instead of
IDisposable), with the DisposeAsync method as follows:

public async ValueTask DisposeAsync()
{
    gatewayClient = new ClosedGatewayClient();
    await channelToGateway.ShutdownAsync();
}

This allows appropriate async disposal using await using statements or even just await zeebeClient.DisposeAsync();. It does, however, present the challenge of implementing IAsyncDisposable while still maintaining netstandard2.0 support.

@0x15e 0x15e added the bug Something isn't working label Apr 2, 2020
@ChrisKujawa
Copy link
Collaborator

Thanks @0x15e for reporting I will try to take a look at it next week.

@bulldetektor
Copy link
Contributor

I believe this PR will fix this; #104

As far as I could see, the problem isn't actually the Wait-mehod on ShutdownAsync(), but the fact that Dispose is called twice. And the second call will call ShudownAsync() on a ClosedGatewayClient, which then will throw.

@0x15e
Copy link
Author

0x15e commented Apr 3, 2020

@bulldetektor If it was just throwing that would be one thing, but no exception comes out of that call that I've been able to catch. Even when I have the debugger set to break on all exceptions, it doesn't catch anything on the Dispose call. It just terminates.

@bulldetektor
Copy link
Contributor

bulldetektor commented Apr 3, 2020

@0x15e Not all exceptions are "catchable", but I belive you could try to set up an eventhandler for any unhandled exceptions and see if you can put a breakpoint there. Something like this;

static async Task Main(string[] args)
{
    System.AppDomain.CurrentDomain.UnhandledException += UnhandledExceptionTrapper;
    TaskScheduler.UnobservedTaskException += UnobservedTaskException;
    
    // rest of your code...
}
static void UnhandledExceptionTrapper(object sender, UnhandledExceptionEventArgs e) {
        Console.WriteLine(e.ExceptionObject.ToString());
        Console.WriteLine("Press Enter to continue");
        Console.ReadLine();
        Environment.Exit(1);
}
static void UnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e)
{
    // do something
}

@0x15e
Copy link
Author

0x15e commented Apr 3, 2020

@bulldetektor Interesting. TIL.

Well you guys know the code better than I do so I'll leave the fixing to you.

@bulldetektor
Copy link
Contributor

@0x15e I'm in no way entitled to claim that I know this code base, and I agree with you that it would be nice to to have the "async dispose" of netstandard2.1. But that would be a pretty heavy breaking change, and so should probably be done by multi-targeting both netstandard 2.0 and 2.1. But I'll leave that decision to @Zelldon :)

@ChrisKujawa
Copy link
Collaborator

I would assume this is fixed, at least I can't reproduce it with the given example.

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

3 participants