Seen while going over bug 231418 IOCache::read and IOCache::write are called with a WorkQueue as parameter [1] In Engine::readFile [2] and Caches::retrieveOriginFromDirectory [3]; it is called with the main WorkQueue which is a serial work queue In Storage::dispatchReadOperation [4] however it is called using the ioQueue which is the a concurrent one, only to be called again using the main thread one in Storage::traverse [5] The use of a concurrent WorkQueue is dangerous as there's no guarantee in the order of execution of the tasks queued, and they could run simultaneously on different thread. The glibc implementation in particular, doesn't guarantee that the completion handler will be called in the right order should there be multiple call to read. For now however, the glibc WorkQueue is always a serial one, but should this change in the future, it could cause undefined behaviour. [1] https://webkit-search.igalia.com/webkit/rev/db21dfc9fcff4b8205577497cc74941727528dfb/Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannel.h#52-53 [2] https://webkit-search.igalia.com/webkit/rev/db21dfc9fcff4b8205577497cc74941727528dfb/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp#505 [3] https://webkit-search.igalia.com/webkit/rev/db21dfc9fcff4b8205577497cc74941727528dfb/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp#87 [4] https://webkit-search.igalia.com/webkit/rev/db21dfc9fcff4b8205577497cc74941727528dfb/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp#752 [5] https://webkit-search.igalia.com/webkit/rev/db21dfc9fcff4b8205577497cc74941727528dfb/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp#1000
I think we want to keep a concurrent work queue for cocoa at least.
(In reply to Chris Dumez from comment #1) > I think we want to keep a concurrent work queue for cocoa at least. The API used by the cocoa code blocks and serialises the callback. So we could give to read a serial queue where to run the completion handler which itself queue to the concurrent queue. It would make the code identical to now with minimal performance impact (which remained to be measured imho) but safer for other platforms
In further read; the code and use of the work queue is fine. It is expected for things to run out of order and will continue only once all tasks have run