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

non blocking output reader #1521

Draft
wants to merge 3 commits into
base: nightly
Choose a base branch
from

Conversation

DasBabyPixel
Copy link
Contributor

@DasBabyPixel DasBabyPixel commented Oct 2, 2024

Motivation

Logging had some issues with blocking when a full line wasn't written in output.

Modification

Fixed those issues by implementing a custom readLine() which calls ready() before every read()

Result

Logging works with the new system (and very little delay, which is very nice, as opposed to 1 second polling alternative)

Other context

@0utplay or @GiantTreeLP please review this derklaro is fed up with this problem
Still based on the old logging to better highlight modifications. This is the reason why this is a conflict
As soon as this is reviewed I can rebase/merge and revert the revered new logging system to make this mergeable

@0utplay
Copy link
Member

0utplay commented Oct 3, 2024

Does this catch jvm crashes?

@DasBabyPixel
Copy link
Contributor Author

Does this catch jvm crashes?

Catches invalid jvm arguments
image

Also catches everything in a shutdown hook
image
(Used FileDescriptor, because logging frameworks don't print everything to console, probably some issue with async logging on shutdown)

@DasBabyPixel DasBabyPixel force-pushed the non-blocking-output-reading branch from adcd07b to 8afcbef Compare January 20, 2025 15:18
@DasBabyPixel DasBabyPixel marked this pull request as draft January 20, 2025 15:20
@DasBabyPixel
Copy link
Contributor Author

DasBabyPixel commented Jan 20, 2025

May have some issues after considerable changes, I'm going to use this in my build first and if I think it is stable, I'm going to mark this ready for review.
The reason why the old logging reader stinks: it doesn't catch jvm crashes.

@DasBabyPixel DasBabyPixel force-pushed the non-blocking-output-reading branch from cc73ac4 to ae9e900 Compare January 20, 2025 15:30
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.

2 participants