[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: |
|
||||||||
| 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:
|
| Comments |
| Comment by Githook User [ 24/Jun/22 ] |
|
Author: {'name': 'sergey.galtsev', 'email': 'sergey.galtsev@mongodb.com', 'username': 'brushless-glitch'}Message: |
| 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. |