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

fix:check closed in synchronized block when multi thread. #721

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

songjinze
Copy link
Contributor

@songjinze songjinze commented Jun 14, 2023

Pull Request Description

Should check closed when multi thread in synchronized block so that stream won't be closed twice.

Acceptance Test

  • Building the code with gradle clean build still works.

Questions

  • Does this pull request break backward compatibility?

  • Does this pull request require other pull requests to be merged first?

    • Yes, please see #...
    • No
  • Does this require an update of the documentation?

    • Yes, please see [provide details here]
    • No

@carlspring
Copy link
Owner

@songjinze ,

Thank you very much for your contribution!

Would it at all be possible to provide some sort of test case for this?

@songjinze
Copy link
Contributor Author

@songjinze ,

Thank you very much for your contribution!

Would it at all be possible to provide some sort of test case for this?

@carlspring Just added a test for thread safe close. I'm not sure how to make a test for multi write because it may not be stable for throwing an io exception when one thread is closing and another is writing.

@songjinze songjinze temporarily deployed to external-collaborators July 11, 2023 06:23 — with GitHub Actions Inactive
@songjinze songjinze had a problem deploying to external-collaborators July 11, 2023 06:23 — with GitHub Actions Error
@songjinze songjinze had a problem deploying to external-collaborators July 11, 2023 06:23 — with GitHub Actions Error
@songjinze songjinze had a problem deploying to external-collaborators July 11, 2023 06:23 — with GitHub Actions Error
@songjinze songjinze had a problem deploying to external-collaborators July 11, 2023 06:23 — with GitHub Actions Error
@songjinze songjinze had a problem deploying to external-collaborators July 11, 2023 06:23 — with GitHub Actions Error
@steve-todorov steve-todorov added this to the 1.0.2 milestone Jul 11, 2023
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@songjinze songjinze had a problem deploying to external-collaborators July 16, 2023 09:42 — with GitHub Actions Failure
@steve-todorov
Copy link
Collaborator

Hey @songjinze ,

Sorry for our delay - we've been busy at our daytime work and had to do some pipeline fixes to allow builds to run from contributors which was why we postponed merging this for so long.

We are planning to merge this PR and release 1.0.2 at some point this week or the beginning of next. Don't worry about rebasing / updating your PR with the upstream. We'll do it for you before we merge. :)

@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov steve-todorov temporarily deployed to external-collaborators July 25, 2023 14:05 — with GitHub Actions Inactive
@steve-todorov
Copy link
Collaborator

@songjinze thanks for your contribution! :)

@steve-todorov steve-todorov merged commit 64c5ed8 into carlspring:master Jul 25, 2023
@github-actions github-actions bot mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants