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

Make notice/warn/debug and friends thread safe #4357

Open
ezyang opened this issue Feb 24, 2017 · 10 comments
Open

Make notice/warn/debug and friends thread safe #4357

ezyang opened this issue Feb 24, 2017 · 10 comments

Comments

@ezyang
Copy link
Contributor

ezyang commented Feb 24, 2017

At the moment, they are only as thread safe as a call to hPutStr is; in other words, not very.

The best way to fix this without introducing global mutable state is to finally implement the logging infrastructure for Verbosity that I've been muttering about:

  1. Augment Verbosity with logger functions that it will call when it otherwise wanted to emit to stdout/stderr
  2. Make a new smart constructor for Verbosity that lives in IO, and allocates a logging thread, and initializes the logger function to send a message to the logging thread.
@23Skidoo
Copy link
Member

I actually implemented something like that for the first version of cabal install -j, but then the change was rolled back because the diff was too large/intrusive and Handles are protected by locks.

/cc @dcoutts

@ezyang
Copy link
Contributor Author

ezyang commented Feb 25, 2017

If we put the loggers in Verbosity there shouldn't really be any ripple effects.

@23Skidoo
Copy link
Member

IMO it would make sense to rename Verbosity to something like Logger if this change were to be implemented.

@BardurArantsson
Copy link
Collaborator

Personally, I think it might make more sense to do an MTL-style approach where we have a thin wrapper over IO which grants whatever capabilities are needed in any given piece of "IO" code... but then Verbosity is already plumbed into most places where we do any output[1], so it definitely seems like the least intrusive solution currently. The MTL-style approach has the admittedly very-long term advantage that we could start limiting other things (such as e.g. subprocess creation) with further "capability classes") which would make the code a lot more manageable.

(See also #4126)

@ezyang
Copy link
Contributor Author

ezyang commented Feb 25, 2017

I too would really like to see Cabal have some more monads (though, I quite like concrete but abstract monads ;-) But this would basically involve churning the entire codebase, and there's never quite a good time to do that... (it's even worse for Cabal library since everything stops being BC.)

@BardurArantsson
Copy link
Collaborator

Yeah, agreed. Though, I was thinking that a slow migration might be feasible with MTL-style type classes... (But Cabal would become... 'interesting'.). Realistically it's probably at least a year or two (calendar time) of effort, I should imagine.

AFAICT the Verbosity change you're suggesting doesn't actually make an MTL-style approach any more/less difficult later, so I'm definitely +1 on your suggestion.

@ezyang
Copy link
Contributor Author

ezyang commented Mar 6, 2017

I looked into fixing this to solve #4376 but one thing that struck me is that we probably want to have some global mutable state anyway, to serve as the lock between instances by default. Otherwise, we'll have the unenviable task of tracking down all the places where we generate up a default verbosity and tagging them correctly. (I guess global mutable state doesn't feel too bad here, because stderr and stdout are "global state"?) I wonder what people think...

@BardurArantsson
Copy link
Collaborator

Just a note to self: See also https://www.fpcomplete.com/blog/2017/07/the-rio-monad for potential simple solution for the "break everything" problem.

@carlwr
Copy link

carlwr commented Aug 20, 2024

This problem, i.e. terminal output getting garbled/interleaved, is still present in cabal 3.12.

The problem is not present with -j1, which is expected. The problem is present also with --test-show-details=direct.

The issues #3075, #5756, #4838, #4376 (all open) seemingly describe the same problem.

Illustration

cabal test is used for the illustration below. Most likely the problem is not specific to cabal test however, but can appear with cabal build etc. as well. The problem would then typ. be provoked by having at least one internal library, in order to trigger parallel compilation.

$ cabal clean; cabal test

Resolving dependencies...

[...]

[1 of 1] Compiling Main             ( src/test/t0/Main.hs, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t0/build/t0/t0-tmp/Main.o )
[1 of 1] Compiling Main             ( src/test/t1/Main.hs, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t1/build/t1/t1-tmp/Main.o )
[2 of 2] [Linking2 of  /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t1/build/t1/t1
2] Linking /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t0/build/t0/t0

[...]

Full output:

$ cabal clean; cabal test

Resolving dependencies...
Build profile: -w ghc-9.10.1 -O1
In order, the following will be built (use -v for more details):
 - test5-0.1.0.0 (lib) (first run)
 - test5-0.1.0.0 (test:t1) (first run)
 - test5-0.1.0.0 (test:t0) (first run)
Configuring library for test5-0.1.0.0...
Preprocessing library for test5-0.1.0.0...
Building library for test5-0.1.0.0...
[1 of 1] Compiling Lib              ( src/lib/Lib.hs, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/build/Lib.o, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/build/Lib.dyn_o )
Configuring test suite 't1' for test5-0.1.0.0...
Configuring test suite 't0' for test5-0.1.0.0...
Preprocessing test suite 't1' for test5-0.1.0.0...
Preprocessing test suite 't0' for test5-0.1.0.0...
Building test suite 't1' for test5-0.1.0.0...
Building test suite 't0' for test5-0.1.0.0...
[1 of 1] Compiling Main             ( src/test/t0/Main.hs, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t0/build/t0/t0-tmp/Main.o )
[1 of 1] Compiling Main             ( src/test/t1/Main.hs, /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t1/build/t1/t1-tmp/Main.o )
[2 of 2] [Linking2 of  /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t1/build/t1/t1
2] Linking /Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t0/build/t0/t0
Running 1 test suites...
Test suite t1: RUNNING...
Running 1 test suites...
Test suite t0: RUNNING...
Test suite not yet implemented.
2
Test suite t1: PASS
Test suite logged to:
/Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t1/test/test5-0.1.0.0-t1.log
1 of 1 test suites (1 of 1 test cases) passed.
Test suite not yet implemented.
2
Test suite t0: PASS
Test suite logged to:
/Users/<user>/<projdir>/dist-newstyle/build/aarch64-osx/ghc-9.10.1/test5-0.1.0.0/t/t0/test/test5-0.1.0.0-t0.log
1 of 1 test suites (1 of 1 test cases) passed.

The above run was performed on a simple example project basically identical to the default project, but with two test suites.

Versions used:

cabal --version
# cabal-install version 3.12.1.0
# compiled using version 3.12.1.0 of the Cabal library

uname -mrsv
# Darwin 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:16:51 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T8103 arm64

@ulysses4ever
Copy link
Collaborator

@LeuschkeTressa thanks a lot for rehashing the problem! Indeed, it's embarrassing to not use thread-safe IO in 2024. No one is currently working on this, to my knowledge, so we'd welcome contributions in this field very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants