aboutsummaryrefslogtreecommitdiff
path: root/lib/jobs/requestdata.h
diff options
context:
space:
mode:
authorAlexey Rusakov <Kitsune-Ral@users.sf.net>2022-05-06 22:39:34 +0200
committerAlexey Rusakov <Kitsune-Ral@users.sf.net>2022-05-11 16:17:32 +0200
commit98fdf62391fdc5135d8324476903a4c43345e732 (patch)
tree8737d7aafe049cbeb273e8dc82d4cb579f1e56f5 /lib/jobs/requestdata.h
parent3458fdf7b416fe64b82ec9b99553c7b2aa299e4c (diff)
downloadlibquotient-98fdf62391fdc5135d8324476903a4c43345e732.tar.gz
libquotient-98fdf62391fdc5135d8324476903a4c43345e732.zip
Fix race condition in consumeRoomData()
QCoreApplication::processEvents() is well-known to be a _wrong_ solution to the unresponsive UI problem; despite that, connection.cpp has long had that call to let UI update itself while processing bulky room updates (mainly from the initial sync). This commit finally fixes this, after an (admittedly rare) race condition has been hit, as follows: 0. Pre-requisite: quotest runs all the tests and is about to leave the room; there's an ongoing sync request. 1. Quotest calls /leave 2. Sync returns, with the batch of _several_ rooms (that's important) 3. The above code handles the first room in the batch 4. processEvents() is called, just in time for the /leave response. 5. The /leave response handler in quotest ends up calling Connection::logout() (processEvents() still hasn't returned). 6. Connection::logout() calls abandon() on the ongoing SyncJob, pulling the rug from under onSyncSuccess()/consumeRoomData(). 7. processEvents() returns and the above code proceeds to the next room - only to find that the roomDataList (that is a ref to a structure owned by SyncJob), is now pointing to garbage. Morals of the story: 1. processEvents() effectively makes code multi-threaded: one flow is suspended and another one may run _on the same data_. After the first flow is resumed, it cannot make any assumptions regarding which data the second flow touched and/or changed. 2. The library had quite a few cases of using &&-refs, avoiding even move operations but also leaving ownership of the data with the original producer (SyncJob). If the lifetime of that producer ends too soon, those refs become dangling. The fix makes two important things, respectively: 2. Ownership of room data is now transfered to the processing side, the moment it is scheduled (see below), in the form of moving into a lambda capture. 1. Instead of processEvents(), processing of room data is scheduled via QMetaObject::invokeMethod(), uncoupling the moment when the data was received in SyncJob from the moment they are processed in Room::updateData() (and all the numerous signal-slots it calls). Also: Room::baseStateLoaded now causes Connection::loadedRoomState, not the other way round - this is more natural and doesn't need Connection to keep firstTimeRooms map around.
Diffstat (limited to 'lib/jobs/requestdata.h')
0 files changed, 0 insertions, 0 deletions