On Thu Aug 3 12:57:37 2023 +0000, Rémi Bernon wrote:
I don't see any particular reason not to do it like that. We had some scenarios in Proton where the mapping was transient, while the desktop was destroyed. A new desktop object was created quickly after, re-using the same mapping. The relevant commit message in Proton is:
server: Reuse shared mapping's mmaped pointer and handle mmap failures. Desktop and its shared mapping can be still alive and may be reused after desktop is unlinked via close_desktop_timeout() but before all references and handles are cleared. On the re-use path the mmap(PROT_WRITE) is called on a write-sealed fd which will fail returning MAP_FAILED. It would then be propagated up and dereferenced causing the server to crash. This fixes the crash + makes sure that we can recreate the desktop successfully if needed. This also correctly unmap the shared memory pointers whenever a mapping is destroyed, fixing memory leaks when many threads are created.There's also some possible patches to seal the mapping to make sure only wineserver can write to them. This requires the memory to be mmap writable only once (and future wineserver maps to re-use it).
I'm not sure I understand the "reuse" part. A desktop object be considered alive as long as it has nonzero reference or handle count. In this case, shouldn't the mapping be kept alive as well?
If I understood correctly, Proton is basically "resurrecting" the mapping if a desktop object is brought back to the namespace. I'm not sure if trying to release the mapping while the desktop is still technically alive in the first place is a good idea. It comes to me as a bit awkward that we don't bind the lifetime of the mapping to the lifetime of the desktop object itself.