Ref: https://codereview.qt-project.org/c/qt/qtbase/+/518039 http://bugreports.qt.io/browse/QTBUG-111855
For Qt 6.6, I undertook a refactor of those two classes to allow the backend to be selected at runtime, instead of compile-time, because Apple in their infinite wisdom[*] decided that App Store applications can't use the System V API. But in the process I've discovered that those APIs are flawed by design. That means they aren't really fixable without a source- and behaviour- incompatible API rewrite. Whether someone decides to do that or not is not important: the APIs as they stand are flawed, racy, and leak memory. Therefore, those classes are dangerous to use and need to be avoided. The change above marks them as deprecated, with no replacement, and schedules them for removal in Qt 7.0 (probably to be moved to a "Qt6CoreCompat" module). Worse, the refactor I attempted for 6.6 *made it worse* for Unix systems, with unavoidable memory leaks. [*] that was sarcastic. I don't think wisdom played a role at all. I think they found it too difficult to secure the API, so they didn't. Q: What's wrong with those classes? A: The API is full of unavoidable race conditions. In Qt 4.4 or 4.5 when Ben and I designed and reviewed those two classes, we attempted to make a common API that would work on Unix and Windows systems, by choosing a simple and common behaviour that would work across the two OS families. But "past us" had much less experience and there are definite flaws that we didn't notice. There are two major flaws in both classes: 1) improperly separate creation from attachment to an existing object. This causes race conditions if two applications attempt to create the object at the same time. It isn't clear what happens: whether the object gets replaced or not, whether attachment happens for the failed creation, etc. QSharedMemory attempts to fix this by using a QSystemSemaphore to control access, but since QSystemSemaphore is racy on its own (and in a worse way), that just moves the goalpost and introduces a second failure point. 2) improperly inform ownership of the object. This is the biggest of the two. This happens because on Windows, the objects disappear when the last user disconnects from them, but not so on Unix systems, so the backends attempt to detect the situation and delete. However, the detection is racy and may choose to delete at the moment that another application attempts to attach (see problem #1). It's unclear what happens on Unix systems if two processes race the deletion or race a deletion with an attachment. To add insult to injury, #2 got worse in Qt 6.6 because the POSIX backend (the one Apple wants us to use and I made default) does not support detecting the last user at all. This means QSharedMemory/POSIX *never deletes* the shared memory object. We needed an API for the user to explicitly choose to delete, but I find that's covering the Sun with a sieve. There's a third, minor flaw in QSharedMemory: 3) QSharedMeory::locked() exists. That suggests to people that this is a good way to write shared memory code. Instead, they should strive to write thread- safe code, and only if it is not possible use a locking primitive. Said locking should be optional. Q: I still need shared memory, what can I use instead? A: Use QFile::map(). A memory-mapped file without the QFile::MapPrivateOption is shared memory. See https://doc.qt.io/qt-6/shared-memory.html#sharing-memory-via-memory-mapped-files Files are well-understood and common across all OSes, which should also make reviewing code for problems and race conditions much simpler. For example, using QTemporaryFile and renaming, you can prepare the shared memory area before committing it to visibility to other processes. Q: I need an equivalent of QSystemSemaphore, what can I use instead? A: Use QFile::map() and put an atomic or a PThread locking primitive there. Alternatively, use QLockFile. Q: But who deletes the file? A: That's up to you to decide for your application. If we could have had a one-size-fits-all solution, we'd have fixed QSharedMemory because this is exactly the nature of the second flaw above (POSIX shared memory objects *are* files on Linux). My recommendation is that there's a central process that creates the file and is always responsible for deleting it. This process must start before and exit after any other processes that attempt to use the object. If you really must know if anyone is still using it, add a QBasicAtomicInt somewhere in it and implement a never-increment-from-zero reference counter in it. Q: What should I use for rendezvousing (i.e., this unclear-shared-ownership model)? A: Here are a couple of suggestions: - a D-Bus well-known bus name - a listening socket - a Q(Temporary)File with a well-known name aside from files, the others automatically clean up on application crashes too Q: My code is working fine for now, I don't want to touch it. What should I do? A: Add #define QT_NO_DEPRECATED_IPC and the deprecation warnings will be silenced. Q: Should we use the new API in Qt 6.6? A: No, they're dead on arrival. Q: Will you work on a replacement API? A: No. QFile suffices for my needs and I've lost interest / motivation in this functionality. Q: Will you accept if someone else works in fixing these classes or providing a replacement? A: Sure. I'll be happy to review your code and offer my insight. I just won't do the coding myself. -- Thiago Macieira - thiago.macieira (AT) intel.com Cloud Software Architect - Intel DCAI Cloud Engineering
smime.p7s
Description: S/MIME cryptographic signature
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
