azure-sdk-for-go icon indicating copy to clipboard operation
azure-sdk-for-go copied to clipboard

Fix deadlock when erroring writes are slow

Open MasslessParticle opened this issue 2 years ago • 9 comments

c.getErr can return before slow writes complete. If there is more than one write and those writes result in errors, the second write blocks on a full errCh. The result is a deadlocked copyFromReader call because the wait groups never clear.

The test included tests demonstrates the deadlock.

This PR causes close to simultaneously drain the errCh while it waits for writes to complete. It combines any errors that are returned.

I looked at the CHANGELOG, but not sure this change fits. Let me know if I need to do something else.

  • [x] The purpose of this PR is explained in this or a referenced issue.
  • [x] The PR does not update generated files.
    • These files are managed by the codegen framework at [Azure/autorest.go][].
  • [x] Tests are included and/or updated for code changes.
  • [ ] Updates to [CHANGELOG.md][] are included.
  • [x] MIT license headers are included in each file.

MasslessParticle avatar Jan 28 '22 19:01 MasslessParticle

Thank you for your contribution MasslessParticle! We will review the pull request and get back to you soon.

msftbot[bot] avatar Jan 28 '22 19:01 msftbot[bot]

CLA assistant check
All CLA requirements met.

@mohsha-msft @seankane-msft Hello all! Is there any word on this PR?

This fixes a goroutine leak that brings down our containers so we're excited to merge a fix and be off a fork. Let me know if I can help!

MasslessParticle avatar Feb 07 '22 14:02 MasslessParticle

Hey @MasslessParticle ,

Thanks a lot for your contribution to our repo. Apologies for I couldn't include your changes in next release 0.3.0. I'll review this PR and include this in release 0.4.0 in March. But in the meantime, I'll merge the changes in an internal azblob dev branch so if you need it, you can pull in that specific branch to your code. Does that sound good to you?

mohsha-msft avatar Feb 07 '22 16:02 mohsha-msft

Thanks for your reply!

No need to merge to a dev branch. I'm excited for the 0.4.0 release

MasslessParticle avatar Feb 08 '22 20:02 MasslessParticle

Hi @MasslessParticle. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

msftbot[bot] avatar Apr 15 '22 10:04 msftbot[bot]

@mohsha-msft is there still interest in accepting this PR?

MasslessParticle avatar Apr 15 '22 13:04 MasslessParticle

Hi @MasslessParticle. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

msftbot[bot] avatar Jul 22 '22 10:07 msftbot[bot]

@jhendrixMSFT @mohsha-msft Is there still interest in this PR?

MasslessParticle avatar Jul 22 '22 14:07 MasslessParticle