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

Configuring a reporter as an empty array create a memory leak #534

Closed
ianwsperber opened this issue Nov 16, 2016 · 1 comment
Closed

Configuring a reporter as an empty array create a memory leak #534

ianwsperber opened this issue Nov 16, 2016 · 1 comment
Labels
bug
Milestone

Comments

@ianwsperber
Copy link
Contributor

@ianwsperber ianwsperber commented Nov 16, 2016

Though obviously an edge case, if a reporter configuration is set to an empty array (essentially blank) then good will write all log messages for that reporter to a stream that is never emptied, resulting in a memory leak. From what I can tell this is due to the check here https://github.com/hapijs/good/blob/master/lib/monitor.js#L128, which should add the NoOp stream according to the condition streamObjs.length <= 1, not streamObjs.length === 1.

I discovered this problem when I configured a dev mode only reporter, which in my confidence file was achieved by setting the reporter to an empty array. It was only in production that enough logs were generated to make the leak evident.

I am putting together a PR for this issue presently. However I will need to confirm this fixes the memory leak prior to creating the PR.

@arb arb added the bug label Nov 17, 2016
@arb arb added this to the 7.1.0 milestone Nov 17, 2016
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Nov 17, 2016

Yeah that sounds right to me. Though I wouldn't technically call it a memory leak... more like a out of bounds buffer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.