JDevlieghere added a comment. In D127922#3595530 <https://reviews.llvm.org/D127922#3595530>, @labath wrote:
> I think we should talk about the threadsafe flag. The reason that the > non-threadsafe mode is safe now is because we're using unbuffered streams, > where each stream operation will map to a single write syscall (which will > then be serialized by the OS somehow). And since we pre-format the log > messages into a temporary buffer, we shouldn't even get overlapping messages > in the output. In fact, I would say that, before this, the threadsafe mode > was mostly useless. > > All of that goes out the window once you start doing arbitrary logic in place > of log writing. As they're implemented now, neither BufferedLogHandler nor > RotatingLogHandler are thread-safe. (Making the rotating handler thread safe > (without locks) should be relatively easy -- assuming we're willing to accept > races between printing messages, and new messages coming in -- but I don't > think that the Buffered handler can be made safe without locking at least the > flushing part). An it's not just that the messages may be slightly wonky. I > think it wouldn't be hard to crash the process by spewing log messages from > multiple threads (and we're pretty good at spewing :P). > > So, what I'd recommend is to drop the thread-safe flag, and make the locking > strategy the responsibility of the individual log handler. When writing to an > unbuffered stream, we wouldn't need any locks at all. The circular handler > could either lock into that or, if you're into that sort of thing, use > atomics to make enqueueing lock-free. The buffered handler could either use a > buffered ostream, wrapped in a mutex, or do something similar to the circular > one, while locking only the flushing code. Yup that makes sense to me. I'll put up a patch for that tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127922/new/ https://reviews.llvm.org/D127922 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
