[Development] Deprecating QSharedMemory and QSystemSemaphore

[Thiago Macieira]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5163 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20231116/3ceaed9b/attachment.bin>