Details
-
Bug
-
Resolution: Fixed
-
P3: Somewhat important
-
5.15, 6.2, 6.5, 6.6.0 FF, dev
-
None
-
-
1
-
3ffc1f977 (dev), 4ea9f75b1 (6.6), a6b47b7b1 (6.5), c821c1f4a (tqtc/lts-5.15), 3981f5e20 (tqtc/lts-6.2), 06d520dbe (dev), b933a5668 (dev), c9fe14640 (6.6), 0188af38b (6.6), 737d3bd74 (6.5), 679a685d0 (6.5), c62c63991 (tqtc/lts-6.2)
-
Foundation Sprint 85, Foundation Sprint 86
Description
The code that counts down the value of QT_FATAL_CRITICALS exhibits a data race. It's not UB, because the variables are atomics, but it nonetheless can lead to no qCritical()'s terminating the application, even though some N > 1 was requested:
// it's fatal if the current value is exactly 1, // otherwise decrement if it's non-zero return fatalCriticals.loadRelaxed() && fatalCriticals.fetchAndAddRelaxed(-1) == 1;
This code must be thread-safe, obviously, but since the checks are separate atomic operations, they can interleave when two threads execute them concurrently (they might even be reordered, since they're relaxed atomic, IIRC). I'm hard-pressed finding an execution that can fail to detect zero, but the relaxed atomics are funny that way, so it's better to change this into a CAS loop, or count down past the 0 and use <= 1.
Attachments
For Gerrit Dashboard: QTBUG-115062 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
489543,3 | QLogging: DRY isFatal(QtMsgType) | dev | qt/qtbase | Status: MERGED | +2 | 0 |
489544,4 | Make sure we don't count down past 0 QT_FATAL_CRITICALS | dev | qt/qtbase | Status: MERGED | +2 | 0 |
489545,3 | QLogging: add qYieldCpu() to CAS loop | dev | qt/qtbase | Status: MERGED | +2 | 0 |
489572,2 | QLogging: DRY isFatal(QtMsgType) | 6.5 | qt/qtbase | Status: MERGED | +2 | 0 |
489573,2 | QLogging: DRY isFatal(QtMsgType) | 6.6 | qt/qtbase | Status: MERGED | +2 | 0 |
489574,2 | QLogging: DRY isFatal(QtMsgType) | tqtc/lts-5.15 | qt/tqtc-qtbase | Status: MERGED | +2 | 0 |
489575,2 | QLogging: DRY isFatal(QtMsgType) | tqtc/lts-6.2 | qt/tqtc-qtbase | Status: MERGED | +2 | 0 |
489730,2 | Make sure we don't count down past 0 QT_FATAL_CRITICALS | 6.6 | qt/qtbase | Status: MERGED | +2 | 0 |
489731,2 | Make sure we don't count down past 0 QT_FATAL_CRITICALS | 6.5 | qt/qtbase | Status: MERGED | +2 | 0 |
489732,2 | QLogging: add qYieldCpu() to CAS loop | 6.6 | qt/qtbase | Status: MERGED | +2 | 0 |
489733,2 | QLogging: add qYieldCpu() to CAS loop | 6.5 | qt/qtbase | Status: MERGED | +2 | 0 |
489736,2 | Make sure we don't count down past 0 QT_FATAL_CRITICALS | tqtc/lts-6.2 | qt/tqtc-qtbase | Status: MERGED | +2 | 0 |
489737,2 | Make sure we don't count down past 0 QT_FATAL_CRITICALS | tqtc/lts-5.15 | qt/tqtc-qtbase | Status: MERGED | +2 | 0 |