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

Possible NullPointerException in DelegatingAsyncDisruptorAppender when flushing appenders #455

Closed
brenuart opened this issue Dec 27, 2020 · 3 comments
Milestone

Comments

@brenuart
Copy link
Collaborator

The DelegatingAsyncDisruptorAppender may throw a NullPointerException at line https://github.com/logstash/logstash-logback-encoder/blob/master/src/main/java/net/logstash/logback/appender/DelegatingAsyncDisruptorAppender.java#L69 in the appender's output stream is null.

This may appear very odd, but appenders like the RollingFileAppender may have a null output stream during rollover or even after if it failed to rotate the underlying file... I'm pretty sure the FileAppender will throw an exception before the DelegatingAsyncDisruptorAppender had a chance to flush at the end of the batch - so there is no point in checking for a null output stream. At least not until the FileAppender is "fixed" and can handle the null case gracefully...

@philsttr
Copy link
Collaborator

so there is no point in checking for a null output stream

So you're saying don't check for a null outputstream in DelegatingAsyncDisruptorAppender? What would the fix be then?

@brenuart
Copy link
Collaborator Author

That's not what I meant... I should have say "... it seems there is no point in checking for a null output stream because it is likely an exception would be thrown earlier anyway...

The OutputStreamAppender#getOutputStream() doesn't say anything about whether it may or not return a null output stream. IMHO it would be safer to check for null anyway.

My example around the RollingFileAppender is from experience. The output stream can be null for a very short time during rollover (fair enough) - but it can stay null for longer when it fails to roll. In this case the appender is usually broken and won't be able to recover by itself.

Sorry for not being clear the first time :-(

@philsttr
Copy link
Collaborator

Closed by #457

@philsttr philsttr added this to the 6.6 milestone Dec 30, 2020
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

No branches or pull requests

2 participants