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

Closed LoggerSet behaves confusingly #166

Open
zudov opened this issue Jun 15, 2018 · 1 comment
Open

Closed LoggerSet behaves confusingly #166

zudov opened this issue Jun 15, 2018 · 1 comment

Comments

@zudov
Copy link

zudov commented Jun 15, 2018

Of course, LoggerSet isn't supposed to be usable after it was closed (with rmLoggerSet), however it should fail in a way that doesn't cause confusion.

I use this utility to test the current behaviour:

withClosedLogger f = do
  logger <- newStdoutLoggerSet defaultBufSize
  rmLoggerSet logger
  f logger

Trying to flush/close an already closed logger:

> withClosedLogger flushLoggerSet
*** Exception: thread blocked indefinitely in an MVar operation
> withClosedLogger rmLoggerSet
*** Exception: thread blocked indefinitely in an MVar operation

That is if you are lucky. Actually it usually takes it some time blocking after which it throws an exception (or never). It took me some time to figure out that our test suite silently hangs because it tries to close the logger twice.

Trying to write to an already closed logger:

> withClosedLogger (`pushLogStrLn` toLogStr "nope")
>

That is, nothing happens.

Better behavior

To avoid confusion all operations on a closed LoggerSet should be either a no-op (do nothing just return) or an instant, deterministic failure (throwIO LoggerSetIsClosed). I think that:

  • Flushing a closed logger should be a no-op. Conceptually we are asked to flush buffered messages and there's nothing to flush, so we should just return.
  • Closing a closed logger should be a no-op or a failure. Making it an explicit failure would bring a potential lifecycle issue to user's attention. On the other hand it would hardly hurt if the disposable resources are disposed twice. In many cases one could want to do that from several places to "really make sure" that their resources are released. I'd prefer if repeated rmLoggerSet would just silently return
  • Writing to a closed logger should be a failure. If there's a lifecycle problem and something is logged after the logger has been closed it shouldn't silently succeed loosing the important messages. It should throw, so that this problem is discovered and fixed before those messages are actually needed.
@Abastro
Copy link

Abastro commented May 31, 2022

Uh, just got bitten by this. First time ever hoping for broader linear type adoption.

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