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(java): log messages may not flush by app exit #3028

Merged
merged 3 commits into from
Oct 11, 2021
Merged

Conversation

RomainMuller
Copy link
Contributor

In some cases, messages sent by JavaScript using console.log and
similar may not be completely flushed by the time the Java thread that
forwards that data is closed. This is because the thread might not have
been given a chance to run when this.stop is set to true, which makes
it exit immediately.

This change adds a new step that triggers once this.stop becomes
true that will flush the inputStream until EOF in a blocking way.
When this happens, the child process has already been requested to exit,
and so EOF should be reached relatively quickly.

Fixes #3009


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In some cases, messages sent by JavaScript using `console.log` and
similar may not be completely flushed by the time the Java thread that
forwards that data is closed. This is because the thread might not have
been given a chance to run when `this.stop` is set to true, which makes
it exit immediately.

This change adds a new step that triggers once `this.stop` becomes
`true` that will flush the `inputStream` until EOF in a blocking way.
When this happens, the child process has already been requested to exit,
and so EOF should be reached relatively quickly.

Fixes #3009
@RomainMuller RomainMuller requested a review from a team October 5, 2021 11:55
@RomainMuller RomainMuller self-assigned this Oct 5, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 5, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 5, 2021

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2021

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Oct 11, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2021

Merging (with squash)...

@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2021

Merging (with squash)...

@mergify mergify bot merged commit 41631e7 into main Oct 11, 2021
@mergify mergify bot deleted the rmuller/java-eof branch October 11, 2021 09:30
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logs do not flush before Java exit
2 participants