On Sat Jul 1 14:15:57 2023 +0000, Jinoh Kang wrote:
Is there a reason why the section (mapping) object should be responsible for managing the server-side VMA's lifecycle? How about just returning mmap()-ed pointer for the section object on demand, and let the user free it? For example, the desktop object doesn't really need reference to the section object itself; it merely has to access the actual shared memory.[^note] [^note]: At least this was the impression I got from https://gitlab.winehq.org/rbernon/wine/-/commits/mr/shared-memories. Maybe there's a use case for resizing the mapping, but I suppose that requires creating a new replacement anyway, and the object can be referenced by name in that case.
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).