WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
48520
[Qt][WK2] SharedMemory is broken
https://bugs.webkit.org/show_bug.cgi?id=48520
Summary
[Qt][WK2] SharedMemory is broken
Balazs Kelemen
Reported
2010-10-28 06:49:58 PDT
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.
Attachments
proposed patch
(2.05 KB, patch)
2010-10-28 06:58 PDT
,
Balazs Kelemen
no flags
Details
Formatted Diff
Diff
qsharedmemory based implementation
(6.80 KB, patch)
2010-11-02 02:38 PDT
,
Balazs Kelemen
no flags
Details
Formatted Diff
Diff
Patch
(6.90 KB, patch)
2010-11-02 06:55 PDT
,
Balazs Kelemen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Balazs Kelemen
Comment 1
2010-10-28 06:58:12 PDT
Created
attachment 72185
[details]
proposed patch
Andreas Kling
Comment 2
2010-10-28 07:46:35 PDT
Comment on
attachment 72185
[details]
proposed patch r=me
Balazs Kelemen
Comment 3
2010-10-28 07:52:17 PDT
Comment on
attachment 72185
[details]
proposed patch Clearing flags on attachment: 72185 Committed
r70776
: <
http://trac.webkit.org/changeset/70776
>
Balazs Kelemen
Comment 4
2010-10-28 07:52:25 PDT
All reviewed patches have been landed. Closing bug.
Balazs Kelemen
Comment 5
2010-11-01 14:28:50 PDT
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
Comment 6
2010-11-01 15:07:32 PDT
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
Comment 7
2010-11-02 02:06:55 PDT
***
Bug 48619
has been marked as a duplicate of this bug. ***
Balazs Kelemen
Comment 8
2010-11-02 02:38:19 PDT
Created
attachment 72641
[details]
qsharedmemory based implementation Fix almost all crashes in run-webkit-tests -2
Balazs Kelemen
Comment 9
2010-11-02 06:55:11 PDT
Created
attachment 72659
[details]
Patch Doing key generation more safely by the use of UUuid.
Andreas Kling
Comment 10
2010-11-02 07:06:29 PDT
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
Comment 11
2010-11-02 08:49:11 PDT
Committed in
http://trac.webkit.org/changeset/71118
.
WebKit Review Bot
Comment 12
2010-11-02 09:45:52 PDT
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
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug