Port PlatformSharedMemoryRegion to OS/2

[dmik]

Currently we use the POSIX code path it implements shared memory via mmap on a temporary file (to be able to access it across different processes). mmap for OS/2 is just a rough emulation and it has its limitations here and there, especially in case of file-based mappings. Chromium spits warnings end errors like:

[2164:1:0729/221359.025000:WARNING:platform_shared_memory_region_posix.cc(25)] unlink: Permission denied (13)
[2164:5:0729/221400.431000:FATAL:shared_memory_tracker.cc(67)] Check failed: usages_.find(mapping.raw_memory_ptr()) == usages_.end(). 

Given that we have a native API for shared memory which is much simpler to use and has better ways of sharing it across processes, it makes no sense to use emulation. Should be done in native.

Needed for https://github.com/bitwiseworks/qtwebengine-os2/issues/4.

[dmik]

This task is a bit bigger. According to comments in the source, PlatformSharedMemoryRegion is a refactor of the old SharedMemoyHandle code, see https://bugs.chromium.org/p/chromium/issues/detail?id=795291 for details.

Although SharedMemoyHandle doesn't seem to be used in new code, I have to port it too as otherwise it doesn't link.

[dmik]

From what I see, Chromium wants to map the same shared memory object both in read/write and in R/O but OS/2 doesn't support that out of the box as there is no concept of a "mapping" at all — the memory object is represented by its virtual address and it's the same in all processes.

We may use DosAliasMem to map the same memory object in r/o in a given process and it seems to work. But Chromium also seems to request multiple mappings of the same object and expects them to have different addresses as otherwise this assertion fails:

[27856:5:0808/154549.346000:FATAL:shared_memory_tracker.cc(67)] Check failed: usages_.find(mapping.raw_memory_ptr()) == usages_.end().

This looks like a waste of address space. I need to analyze more how this stuff is used across the code to see if we have to follow this assertion or may ignore it. Aliasing the same memory over and over again looks like a waste of virtual address space to me. Perhaps it's not a problem in a 64-bit world but it is in our 32-bit world.

[dmik]

Another observation is that Chromium wants access to shared memory again after all previous mappings are released. Given that we supply the shared memory address itself as an R/W mapping, calling DosFreeMem on that mapping later unmaps it from the virtual address space of the process. If it's the only process using it, memory will be also released back to the system. An attempt to map this memory again will expectedly fail in both cases.

In order to solve that problem we could introduce memory "handles" (like file mapping handles on Win32) with some additional information attached to them (like the reference counter, etc). But this looks like an overkill given our timeframe as it requires some global inter-process structure to track these handlers with an API exposed via LIBCx (or LIBCn) -- it's several days of work minimum.

Instead, I will try to go the other way. Simply allocate it again if it's freed — the size of the region is known as it is stored in the Chromium-side memory region structure.

[dmik]

Second thought: if the region gets freed and then allocated again, the data will be lost. This might be not what the caller expects. As far as I get it, memory handles should guarantee data persistence until closed. If not using reference counting, the only solution is never free the shared region at all.

We could go a different way and free memory when the handle is "closed" by Chromium (after which it's legally invalid and can't be used for mapping) but given that we use aliases for R/O mappings, this will not work as freeing the original object automatically invalidates any aliases while Chromium expects that mappings are valid (until unmapped) even after the respective handle is closed. This can be rather easily implemented once we get reference counting but it's a task on its own (see above).

For now I think I will just go with never-freed memory to see where it gets us.

[dmik]

The above commit is rather huge and is result of nearly a month of work (besides other things of course). It comes as a whole since it's an "atomic" change. I'm not closing this ticket right now since this version of PlatformSharedMemoryRegion is leaky (as warned above) — it never frees (shared) memory allocated for the purposes of Chromium. But this memory is of course automatically freed by the system as soon as all Chromium processes using it end. Given that we work in QTWEBENGINE_CHROMIUM_FLAGS=--single-process mode for now, it even involves a single process. Quite acceptable for further debugging and even limited use.

When we enable multiple renderer processes (and/or are ready for production), we might want to fix this leak. In order to do that, we will need our own shared memory API that will use memory handles instead of direct memory addresses to pass memory around and between processes (just like Windows does). Such a separation will let us associate additional information with each allocated memory object (such as reference counting etc.) that is necessary to free it when it's no longer in use. Note that this will have to be done in LIBCx since handles need to be system wide rather than process wide and LIBCx already provides internal global and process-specific data structures for that.

Also note that fixing this memory leak makes sense even in single process mode — to e.g. actually free unneeded memory when a browser tab closes. But it's not a critical issue that may be postponed for now as implementing a LIBCx API is not a quick task and it's more important to make browsing more or less stable first (with #21 being the next major obstacle of that target).

[dmik]

JFYI, this comment contains some Chromium related info related also to this ticket https://github.com/bitwiseworks/libcx/issues/83#issuecomment-701464459:

As for Chromium, looks like I was right too. Judging by comments in this bug https://bugs.chromium.org/p/chromium/issues/detail?id=789959 and by the actual code that uses read-only handles, Chromium seems to use this whole make read-only handles from read-write handles concept as a security measure to protect from various kinds of attacks to the privileged browser process. But as this bug suggests, this protection is only needed on the process boundary, not within the process itself. I.e. Chromium first allocates a read-write memory object in the "main" process and then, when it wants to share it with some other, unprivileged process, it creates a read-only duplicate of that handle and passes it on to that unprivileged process to ensure that the process won't be able to write to that memory. This also explains why I don't see any calls to this "make read-only duplicate" machinery in my logs — I'm working in single process mode for now, so handles are not passed between processes and this machinery is not involved.

And so on.

[dmik]

Hi there, I'm writing this comment from Qt SimpleBrowser on OS/2. Meaning that with the new shmem API from https://github.com/bitwiseworks/libcx/issues/83 it is much more stable, no out of memory conditions so far. Congrats! Overall shared memory usage for this single GitHub tab is just around 140 MB, according to Theseus - not that bad as I previously thought and pretty much tolerable. Further details are in the LIBCx ticket.

[dmik]

With the above commit, Chromium is pretty much usable. I'm closing this issue as the original goal is reached. Any further bugs will be served in separate issues.