Skip to content

Simple handler: evaluate message before taking lock#49

Merged
hvr merged 1 commit intohaskell-hvr:masterfrom
aspiwack:seq-before-lock
Oct 7, 2019
Merged

Simple handler: evaluate message before taking lock#49
hvr merged 1 commit intohaskell-hvr:masterfrom
aspiwack:seq-before-lock

Conversation

@aspiwack
Copy link
Copy Markdown
Contributor

Because of laziness, the msg string in streamHandler can contain
arbitrary computations. In the previous state of affairs, the handler
would first take the lock then putStrLn the message. So all
the (pure) computation would happen in the critical section, and it
couldn't guarantee timely release of the lock.

Calling deepseq before the lock is taken makes sure that the locked
section does only what it must do: print the string.

@aspiwack
Copy link
Copy Markdown
Contributor Author

Any news? This is a rather import bug.

@hvr
Copy link
Copy Markdown
Collaborator

hvr commented Sep 15, 2019

Btw, this branch has currently Git conflicts

Because of laziness, the `msg` string in `streamHandler` can contain
arbitrary computations. In the previous state of affairs, the handler
would first take the lock then `putStrLn` the message. So all
the (pure) computation would happen in the critical section, and it
couldn't guarantee timely release of the lock.

Calling `deepseq` before the lock is taken makes sure that the locked
section does only what it must do: print the string.
@aspiwack
Copy link
Copy Markdown
Contributor Author

@hvr the patch is now rebased (for the curious onlooker: the conflicts were in the cabal dependencies, and the import lists)

@hvr hvr merged commit 8375615 into haskell-hvr:master Oct 7, 2019
@hvr
Copy link
Copy Markdown
Collaborator

hvr commented Oct 7, 2019

Thanks! This got released as part of http://hackage.haskell.org/package/hslogger-1.3.1.0

@aspiwack aspiwack deleted the seq-before-lock branch October 9, 2019 08:53
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