Bug 48520 – [Qt][WK2] SharedMemory is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Balazs Kelemen]

We have implemented SharedMemory with the MappedMemoryPool that was designed to UpdateChunk.
We should not allow the pool to recycle the mapped chunk until it is used at the receiving side.

[Balazs Kelemen]

Created attachment 72185 [details]
proposed patch

[Andreas Kling]

Comment on attachment 72185 [details]
proposed patch

r=me

[Balazs Kelemen]

Comment on attachment 72185 [details]
proposed patch

Clearing flags on attachment: 72185

Committed r70776: <http://trac.webkit.org/changeset/70776>

[Balazs Kelemen]

All reviewed patches have been landed.  Closing bug.

[Balazs Kelemen]

My assumtion was wrong. m_data is used after we have passed the handle at least there:
VisitedLinkProvider.cpp:122
-------
RefPtr<SharedMemory> currentTableMemory = m_table.sharedMemory();
...
const LinkHash* currentLinkHashes = static_cast<const LinkHash*>(currentTableMemory->data());
for (unsigned i = 0; i < currentTableSize; ++i) {
    LinkHash linkHash = currentLinkHashes[i];
-------

This caused crashes on our wk2 bot: http://webkit.sed.hu/buildbot/waterfall?show=x86-32%20Linux%20Qt%20Release%20WebKit2
To correctly fix it we need am additional member in SharedMemory (a bool for example).

[Balazs Kelemen]

After running some tests I have realized that a bool is not enough
(tests had been timed out instead of crashing with the fix).
The ownership of the shared memory is shared across processes so
we need something that acts as a handle. Ideally we should use
QSharedMemory but as antti noted it has some problems on MAC:
http://bugreports.qt.nokia.com/browse/QTBUG-9446.

[Balazs Kelemen]

*** Bug 48619 has been marked as a duplicate of this bug. ***

[Balazs Kelemen]

Created attachment 72641 [details]
qsharedmemory based implementation

Fix almost all crashes in run-webkit-tests -2

[Balazs Kelemen]

Created attachment 72659 [details]
Patch

Doing key generation more safely by the use of UUuid.

[Andreas Kling]

Comment on attachment 72659 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=72659&action=review

This is beautiful, r=me!

As discussed on IRC, if the QUuid generation turns out to be too expensive, we can always replace it by something more light-weight later on.

> WebKit2/Platform/qt/SharedMemoryQt.cpp:134
> +    // m_impl must be not null and it must points to a valid QSharedMemory object.

s/not null/non-null/
s/points/point/

[Balazs Kelemen]

Committed in http://trac.webkit.org/changeset/71118.

[WebKit Review Bot]

http://trac.webkit.org/changeset/71118 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
fast/text/international/plane2.html