A cancellation doesn't work in some cases and app hangs.

[gurustron]

I've played a little bit with an issue raised here it seems that without setting NamedPipeClientStream to PipeOptions.Asynchronous after disposing stream it endlessly waits in PipeStream.Read for local Windows connection.

Steps to repro:

create console app (NET71, .NETCORE) on Windows machine with empty Docker.

    public static async Task Main(string[] args)
    {
        var cts = new CancellationTokenSource();

        var _client = new DockerClientConfiguration().CreateClient();
        cts.CancelAfter(3000);
        cts.Token.Register(() => { Console.WriteLine("Cancel happened"); });
        var task = _client.System.MonitorEventsAsync(new ContainerEventsParameters(), new Progress(), cts.Token);
        await task.ConfigureAwait(false);
        Console.WriteLine("finished");
        Console.ReadKey();
    }
    
    class Progress : IProgress<JSONMessage>
    {
        internal Action<JSONMessage> _onCalled;

        void IProgress<JSONMessage>.Report(JSONMessage value)
        {
            _onCalled(value);
        }
    }

[msftclas]

All CLA requirements met.

[marcelltoth]

I'm running into the same issue.

It looks like microsoft kind of ignores this repo, in the meanwhile does anyone have a good workaround? (Other than just keeping all my tasks alive, which does not sound great.)

[dgvives]

Hello @gurustron
I'm going through all unmerged PR, my intention is to create a PR with code compatible with latest Docker API.
There are outdated models in current definition and this is causing automatic tests to fail within the CI/CD pipeline, causing PR to not be even reviewed.

Would you check if MonitorEventsAsync is working? I haven't managed to

[jterry75]

@dgvives - Unrelated to this PR but if you are going to do that I would prefer that we just remove tools/specgen anymore and just manually update the models. Docker changed how all of this worked long ago when the broke the branches into EE/CE and made specgen less than helpful. I'm all for updating the models I just think that specgen is not worth the effort anymore. I no longer have the time to really contribute here but I am happy to merge PR's if you want to do this work. Thanks!

[galvesribeiro]

Yeah, that is what I’m working into. A client that doesn’t depends on Go anymore. Sorry if it is taking too long. Life is being hard recently... My idea is to generate things based on the yaml from the API with Roslyn. Still working on it... Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Justin <notifications@github.com> Sent: Monday, January 4, 2021 2:22:30 PM To: dotnet/Docker.DotNet <Docker.DotNet@noreply.github.com> Cc: Subscribed <subscribed@noreply.github.com> Subject: Re: [dotnet/Docker.DotNet] A cancellation doesn't work in some cases and app hangs. (#351) @dgvives<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdgvives&data=04%7C01%7C%7C7c614e6a5d4c45aacd4708d8b0d55193%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637453777528915147%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9XE%2BVdOwqDkfdkt3Uy9j8QRIt6G6kZBp%2F%2BcfDiue0mo%3D&reserved=0> - Unrelated to this PR but if you are going to do that I would prefer that we just remove tools/specgen anymore and just manually update the models. Docker changed how all of this worked long ago when the broke the branches into EE/CE and made specgen less than helpful. I'm all for updating the models I just think that specgen is not worth the effort anymore. I no longer have the time to really contribute here but I am happy to merge PR's if you want to do this work. Thanks! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2FDocker.DotNet%2Fpull%2F351%23issuecomment-754104913&data=04%7C01%7C%7C7c614e6a5d4c45aacd4708d8b0d55193%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637453777528925141%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=zVYArtXkkERQ%2F4FsU%2BO9T%2BFmips0WI3pYo91UDdxXLY%3D&reserved=0>, or unsubscribe<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABD64OE65QJUOXSCE4L6J7LSYH2NNANCNFSM4F3UFFHA&data=04%7C01%7C%7C7c614e6a5d4c45aacd4708d8b0d55193%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637453777528925141%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=CAkIgGoTi5aTFgXC81MkVzJrQw5CRJGJzRHbuHfdygA%3D&reserved=0>.

[dgvives]

@jterry75 @galvesribeiro Some of the models overridden on modeldefs.go are missing the required rest tag for correct model generation. I've updated some I found broken, but not all of them.

Code generation using specgen is what I've used. All the generated models are based on specgen details, as I think they should be at least for now.

Continue on #480

[gurustron]

@dgvives I will try to take a look into it this week.

[dgvives]

@gurustron I confirm these changes have been incorporated in the last merge, MonitorEventsAsync can be cancelled
Current solution is based on #343 using an extension method and throwing an exception whenever token gets cancellated.

However, this behaviour is not the most appropriate as it produces slightly different results when running tests on different environments. I expect to submit a PR with a different approach to this, and include references to yours and others' work as contributor