[Webkit-unassigned] [Bug 51984] [WK2][Qt] Multiple problems with MemoryMappedPool

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=51984





--- Comment #2 from Kimmo Kinnunen <kimmo.t.kinnunen at nokia.com>  2011-01-10 08:09:29 PST ---
(In reply to comment #1)

> > This needs the ConnectionQt.cpp to be modified to use unix socket api or FIFO api to pass all the data. The Qt abstraction of QLocalServer and QLocalSocket cannot express sending FDs. However, I don't see this as a bad thing -- it should just remove unnecessary stuff.
> 
> Sending the descriptor is just sending an integer. This is obviously possible with the current connection infrastructure.

I'm no expert, but I don't think it is.

You need to send the message by using specific API.
In other words: the file descriptor table is not shared after fork. It's only copied during fork. AFAIK.

"File Descriptor Passing" in this url:
http://www.wsinnovations.com/softeng/articles/uds.html


> The memory chunk is used by both process. The role of the isFree member is that it is seen in both process at the same time.
> Having a list in both processes is not enough here. About the race condition: could you give an example when smg wrong happen?

0) Web process has passed 1 update chunk to ui process. Ui process has processed that update chunk and marked the underlying MappedMemory free. Both processes have this MappedMemory in the m_pool.

1) Both processes run MappedMemoryPool::mapMemory(size_t size)

2) Both processes run MappedMemoryPool::isFree() at the same time for the same pool entry. Both process get result 'true'.

3) Both processes run MappedMemoryPool::markUsed() at the same time

I don't see any "atomic compare and increment" operator used. I don't know if they even work cross-process.

> As I see the invariant here is only one of the processes is allowed to write it at a time. The only race condition is when the UI process set it
> to free and the same time the web process reading ifFree but that is a safe situation in all possible outcome.
> 
> 
> > 4) Implementation does not check syscall failures
> > 
> > The implementation should check status of file opens, mmap operations etc.
> 
> Yes, maybe it should. I do not see any rational error handling however, but at least we should do a CRASH() to avoid indeterministic behavior.

No. It should fail the memory allocation. This should fail the update chunk allocation. This should result in empty result in paint. Beats crashing, is easy and somewhat well-defined.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.