Skip to content

A cancellation doesn't work in some cases and a deadlock may occure. … #343

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

A cancellation doesn't work in some cases and a deadlock may occure. … #343

wants to merge 1 commit into from

Conversation

suneption
Copy link

…I added an additional cancellation.

@msftclas
Copy link

msftclas commented Sep 18, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

@suneption suneption closed this Sep 21, 2018
@suneption suneption reopened this Sep 21, 2018
@jterry75
Copy link
Contributor

Thanks for the contribution and adding a test. I don't fully understand why this could deadlock. We register a stream.Dispose() on cancellation which would unblock the ReadLineAsync() right? I don't see any issues with adding your code though.

@gurustron gurustron mentioned this pull request Oct 15, 2018
Closed
@YakhontovYaroslav
Copy link

@jterry75
I`ve encountered this issue too when tried to read event from Docker for Windows (18.03 if that matters).
My understanding is it is hanging somewhere in native code inside ReadLineAsync, trying to find end of line delimiter, but if there is no docker events at all it cannot return to managed world and react to token cancellation.

@dgvives dgvives mentioned this pull request Jan 14, 2021
Merged
galvesribeiro pushed a commit that referenced this pull request Jan 28, 2021
@dgvives
Minimum required changes for working tests (#481)
8e3cf4f 
* Updated models and fixed tests

* Discarded model

* Failing tests on CI/CD

* Debugging tests on CI/CD

* Applied PR suggested changes

* error when running tests on CI/CD pipeline

* Implemented approach from PR #343 for cancelling streamReader

* Tests case is behaving differently while it runs on CI/CD pipeline

* Merged changes from author fork

* Error on CI/CD pipeline due to new Message.Status. Discarted Message.Status condition to validate event monitoring

* Serialization for IList<byte> to base64

* PR suggestions by @galvesriverio

* Updated test for DockerService_IsRunning

* Renamed process name in tests

* Updated version for Nerdbank.GitVersioning

* Updated ProcesName in test

Co-authored-by: timt <[email protected]>
@dgvives dgvives mentioned this pull request Feb 3, 2021
Closed
@dgvives dgvives mentioned this pull request Feb 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@suneption @msftclas @jterry75 @YakhontovYaroslav