[SERVER-43997] allow the crtDebugCallback to be called from within the log system Created: 14/Oct/19 Updated: 20/Mar/20 Resolved: 20/Mar/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Logging |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Gabriel Russell (Inactive) | Assignee: | Henrik Edin |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Sprint: | Dev Tools 2020-03-23 | ||||||||
| Participants: | |||||||||
| Description |
|
Currently if a error handling callback occurs during logging, and the callback tries to log, the process will deadlock. I know this can happen in the crtDebugCallback callback, and I worry that it's possible that it would occur in stack tracing or similar. We should investigate the use recursive locks in our logv2 locking front ends or back ends. Alternatively we could think about an how to create and utilize an alternative lockless logging API for use in such callbacks. |
| Comments |
| Comment by Henrik Edin [ 20/Mar/20 ] |
|
Ok. Closing this as "Won't Fix". If this proves to still be an issue even after |
| Comment by Andy Schwerin [ 20/Mar/20 ] |
|
Supposing that the mutex in question is only guarding access to the log file, and not some machinery involved in the generation of the log entry itself, I think it makes sense to either close this "Won't Fix" or minimally change the system so that you can't log from within the logging system. I could imagine a per-thread flag that is set before entering the logging critical section, and just ignoring all attempts to log when that flag is set. |
| Comment by Henrik Edin [ 20/Mar/20 ] |
|
It is still not safe to log in the CRT debug callback if it is fired during a log system write to cout because it will cause a reentry and we're already holding the write lock. A solution would be to change to a recursive lock here: https://github.com/mongodb/mongo/blob/d302a91c30b2648a2262e4ce737e767a3db8597d/src/mongo/logv2/composite_backend.h#L60 Perhaps only on Windows DEBUG. But I don't like the idea of having different lock types on different builds. An other "solution" is to close this as won't fix as the CRT debug callback should hopefully not be fired from writing to cout anymore. I think it's helpful to keep the log in there if it is called for some other error/assert within the CRT. |