[SERVER-66368] Don't call LOGV2 in the boost logging exception handler Created: 10/May/22  Updated: 29/Oct/23  Resolved: 24/Jun/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.1.0-rc0

Type: Bug Priority: Major - P3
Reporter: Gabriel Marks Assignee: Sergey Galtsev (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on SERVER-63843 Don't allow recursive doLog in synchr... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Security 2022-06-27
Participants:

 Description   

Since boost logging exceptions may occur during doLogImpl, this has the potential to re-enter doLogImpl, which is decidedly not reentry-safe. A couple of possible solutions:

  • Add the log entry to a queue, which is popped by some forever-running handler thread.
  • Don't log, and instead output the log message to stderr.


 Comments   
Comment by Githook User [ 24/Jun/22 ]

Author:

{'name': 'sergey.galtsev', 'email': 'sergey.galtsev@mongodb.com', 'username': 'brushless-glitch'}

Message: SERVER-66368 no reentrancy during logging
Branch: master
https://github.com/mongodb/mongo/commit/09fa08f0d1b4e525b02463aad8d0fc7acc200ff7

Comment by Billy Donahue [ 18/May/22 ]

Something we discussed as a possibility for the recursion elimination, maybe worth writing down.

This exception handler sometimes tries to LOG from within a boost::log::core exception catch which is itself likely occurring within a LOG statement. https://github.com/10gen/mongo/blob/0fdf27b6bb98760750ba41b0c246e04d395f3f2b/src/mongo/logv2/log_manager.cpp#L70

I think a safer way to go is to always rethrow and let the higher level doLogImpl handle any attempt to recover. It would know there are no nested LOG calls active. That is we don't want to take action from deep in the call stack where boost is catching exceptions. We want the whole boost call, and really the whole logging call to be resolved and unwound before deciding what to do with its failure.

Comment by Billy Donahue [ 11/May/22 ]

Regarding the queeu idea, I think it wouldn't need to be served by a handler thread. The logImpl call that threw the exception is still running, it just took a detour and can leave a failure message behind. A top-level logImpl call that catches an internal exception could catch the exception and try once to log something about that exception before returning. Whatever caused the initial exception hopefully won't cause any trouble when logging this second message because it's simpler and more controlled than the initial arbitrary log statement. Handling the queue can perhaps be as simple as just emitting a single message that carries an `ex.what()` string. Really it would just be one message, not a sequence. One logImpl throws: okay. But if the complaint complains? We just have to abort then I'd think.

Generated at Thu Feb 08 06:05:14 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.