summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThiago Macieira <thiago.macieira@nokia.com>2010-12-13 14:38:47 (GMT)
committerThiago Macieira <thiago.macieira@nokia.com>2011-01-26 13:03:46 (GMT)
commit121e2b39043a4ffc6583f250aebb9a3a746076c1 (patch)
tree020ced8caef0b4de14106cd562d8b5618d9b11ae
parent1d2d61f6b1cc2472f3cb28e4158fe12a675517ea (diff)
downloadQt-121e2b39043a4ffc6583f250aebb9a3a746076c1.zip
Qt-121e2b39043a4ffc6583f250aebb9a3a746076c1.tar.gz
Qt-121e2b39043a4ffc6583f250aebb9a3a746076c1.tar.bz2
Improve timer ID safety by using a serial counter per ID.
The high bits of the counter are not used, so we store a serial counter there. This has nothing to do with the serial counter used in nextFreeTimerId, which is there for ABA protection. Reviewed-by: Bradley T. Hughes
-rw-r--r--src/corelib/kernel/qabstracteventdispatcher.cpp30
1 files changed, 24 insertions, 6 deletions
diff --git a/src/corelib/kernel/qabstracteventdispatcher.cpp b/src/corelib/kernel/qabstracteventdispatcher.cpp
index e79f87a..cc08f092 100644
--- a/src/corelib/kernel/qabstracteventdispatcher.cpp
+++ b/src/corelib/kernel/qabstracteventdispatcher.cpp
@@ -137,6 +137,12 @@ void QAbstractEventDispatcherPrivate::init()
// free list). As an added protection, we use the cell to store an invalid
// (negative) value that we can later check for integrity.
//
+// ABA prevention simply adds a value to 7 of the top 8 bits when resetting
+// nextFreeTimerId.
+//
+// The extra code is the bucket allocation which allows us to start with a
+// very small bucket size and grow as needed.
+//
// (continues below).
int QAbstractEventDispatcherPrivate::allocateTimerId()
{
@@ -164,6 +170,8 @@ int QAbstractEventDispatcherPrivate::allocateTimerId()
newTimerId = prepareNewValueWithSerialNumber(timerId, b[at]);
} while (!nextFreeTimerId.testAndSetRelaxed(timerId, newTimerId));
+ timerId &= TimerIdMask;
+ timerId |= b[at] & TimerSerialMask;
b[at] = -timerId;
return timerId;
@@ -174,12 +182,13 @@ int QAbstractEventDispatcherPrivate::allocateTimerId()
// X[timerId] = nextFreeTimerId;
// then we update nextFreeTimerId to the timer we've just released
//
-// The extra code in allocateTimerId and releaseTimerId are ABA prevention
-// and bucket memory. The buckets are simply to make sure we allocate only
-// the necessary number of timers. See above.
-//
// ABA prevention simply adds a value to 7 of the top 8 bits when resetting
// nextFreeTimerId.
+//
+// In addition to that, we update the same 7 bits in each entry in the bucket
+// as a counter. That way, a timer ID allocated and released will always be
+// returned with a different ID. This reduces the chances of timers being released
+// erroneously by application code.
void QAbstractEventDispatcherPrivate::releaseTimerId(int timerId)
{
int which = timerId & TimerIdMask;
@@ -187,12 +196,21 @@ void QAbstractEventDispatcherPrivate::releaseTimerId(int timerId)
int at = bucketIndex(bucket, which);
int *b = timerIds[bucket];
- Q_ASSERT(b[at] == -timerId);
+#ifndef QT_NO_DEBUG
+ // debug code
+ Q_ASSERT_X(timerId == -b[at], "QAbstractEventDispatcher::releaseTimerId", "Timer ID was not found, fix application");
+#else
+ if (timerId != -b[at]) {
+ // release code
+ qWarning("Timer ID %d was not found, fix application", timerId);
+ return;
+ }
+#endif
int freeId, newTimerId;
do {
freeId = nextFreeTimerId;//.loadAcquire(); // ### FIXME Proper memory ordering semantics
- b[at] = freeId & TimerIdMask;
+ b[at] = prepareNewValueWithSerialNumber(-b[at], freeId);
newTimerId = prepareNewValueWithSerialNumber(freeId, timerId);
} while (!nextFreeTimerId.testAndSetRelease(freeId, newTimerId));