WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
87521
characterBreakIterator() is not safe to use reentrantly or from multiple threads
https://bugs.webkit.org/show_bug.cgi?id=87521
Summary
characterBreakIterator() is not safe to use reentrantly or from multiple threads
mitz
Reported
2012-05-25 11:28:18 PDT
characterBreakIterator() is not safe to use reentrantly or from multiple threads
Attachments
Add and deply a NonSharedCharacterBreakIterator class
(17.44 KB, patch)
2012-05-25 11:43 PDT
,
mitz
gustavo
: commit-queue-
Details
Formatted Diff
Diff
Add and deply a NonSharedCharacterBreakIterator class
(17.47 KB, patch)
2012-05-25 11:56 PDT
,
mitz
darin
: review+
Details
Formatted Diff
Diff
Added a code path for when COMPARE_AND_SWAP is not enabled
(2.83 KB, patch)
2012-05-25 16:38 PDT
,
mitz
jberlin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
mitz
Comment 1
2012-05-25 11:29:32 PDT
<
rdar://problem/10189597
>
mitz
Comment 2
2012-05-25 11:43:39 PDT
Created
attachment 144110
[details]
Add and deply a NonSharedCharacterBreakIterator class
Gustavo Noronha (kov)
Comment 3
2012-05-25 11:52:13 PDT
Comment on
attachment 144110
[details]
Add and deply a NonSharedCharacterBreakIterator class
Attachment 144110
[details]
did not pass gtk-ews (gtk): Output:
http://queues.webkit.org/results/12791590
mitz
Comment 4
2012-05-25 11:56:19 PDT
Created
attachment 144118
[details]
Add and deply a NonSharedCharacterBreakIterator class
Darin Adler
Comment 5
2012-05-25 13:30:35 PDT
Comment on
attachment 144118
[details]
Add and deply a NonSharedCharacterBreakIterator class View in context:
https://bugs.webkit.org/attachment.cgi?id=144118&action=review
> Source/WebCore/platform/text/TextBreakIterator.h:112 > + TextBreakIterator* get() const { return m_iterator; }
MIght want to consider the type conversion operator for this instead of an explicit get function.
> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:93 > + bool createdIterator = m_iterator && weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), m_iterator, 0);
The reinterpret_cast<void**> idiom is particularly unpleasant. Might be worth looking into a refactoring to clean that up and put it in one place.
> Source/WebCore/platform/text/wince/TextBreakIteratorWinCE.cpp:247 > + m_iterator = new CharBreakIterator();
I normally omit the () in lines of code like this.
mitz
Comment 6
2012-05-25 14:47:28 PDT
Fixed in <
http://trac.webkit.org/r118568
>.
Jessie Berlin
Comment 7
2012-05-25 16:02:56 PDT
(In reply to
comment #6
)
> Fixed in <
http://trac.webkit.org/r118568
>.
I think this caused 19+ crashes to start happening on the Win WK2 Release bots:
http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r118568%20(18598)/results.html
INVALID_POINTER_WRITE Tid [0x12c4] Frame [0x00]: ntdll!ZwRaiseException PRIMARY_PROBLEM_CLASS: INVALID_POINTER_WRITE BUGCHECK_STR: APPLICATION_FAULT_INVALID_POINTER_WRITE STACK_TEXT: 0031ed6c 6f98ccd5 00019d87 7e1740c0 0031ee44 WebKit!WebCore::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator+0xf [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\text\textbreakiteratoricu.cpp @ 99] 0031ed84 6f98f25e 7e03c014 003e9388 00010000 WebKit!WebCore::CharacterData::parserAppendData+0x75 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\dom\characterdata.cpp @ 80] 0031eda4 6fe5481f 0031ede0 7e190800 0031ee44 WebKit!WebCore::Text::createWithLengthLimit+0xde [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\dom\text.cpp @ 292] 0031ee00 6fe2de9f 0031ee44 00000002 0031ee98 WebKit!WebCore::HTMLConstructionSite::insertTextNode+0x15f [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmlconstructionsite.cpp @ 392] 0031ee50 6fe2e483 0031ee5c 7e038b0e 7e038b0e WebKit!WebCore::HTMLTreeBuilder::processCharacterBuffer+0x38f [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmltreebuilder.cpp @ 2384] 0031ee64 6fe30411 0031ee98 0031ee84 6fe30f23 WebKit!WebCore::HTMLTreeBuilder::processCharacter+0x23 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmltreebuilder.cpp @ 2258] 0031ee70 6fe30f23 0031ee98 7e168f20 7e19405c WebKit!WebCore::HTMLTreeBuilder::processToken+0x61 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmltreebuilder.cpp @ 516] 0031ee84 6fe311a0 0031ee98 7e19405c 7e194000 WebKit!WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken+0x23 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmltreebuilder.cpp @ 477] 0031eebc 6fd89119 7e19405c 7e3b13d8 7e194000 WebKit!WebCore::HTMLTreeBuilder::constructTreeFromToken+0x30 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmltreebuilder.cpp @ 461] 0031eefc 6fd89498 00000000 7e194000 0031ef58 WebKit!WebCore::HTMLDocumentParser::pumpTokenizer+0x119 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmldocumentparser.cpp @ 278] 0031ef0c 6f97cb88 0031ef20 7e3b1380 7e3b13d8 WebKit!WebCore::HTMLDocumentParser::append+0xc8 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\parser\htmldocumentparser.cpp @ 372] 0031ef58 6fd74435 7e3b13d8 05110048 7dfd0000 WebKit!WebCore::DecodedDataDocumentParser::appendBytes+0x58 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\dom\decodeddatadocumentparser.cpp @ 50] 0031ef70 6fa8aa8e 05110048 0001a346 7eeb0808 WebKit!WebCore::DocumentWriter::addData+0x55 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\documentwriter.cpp @ 218] 0031efbc 6f74435c 05110048 0001a346 7eec4700 WebKit!WebCore::DocumentLoader::commitData+0xee [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\documentloader.cpp @ 349] 0031eff4 6fa8afd3 7e3b1380 05110048 0001a346 WebKit!WebKit::WebFrameLoaderClient::committedLoad+0x2c [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webcoresupport\webframeloaderclient.cpp @ 870] 0031f014 6fa8b06e 05110048 0001a346 0001a346 WebKit!WebCore::DocumentLoader::commitLoad+0x93 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\documentloader.cpp @ 322] 0031f02c 6fdaf843 05110048 0001a346 0001a346 WebKit!WebCore::DocumentLoader::receivedData+0x4e [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\documentloader.cpp @ 360] 0031f048 6fc54e25 05110048 0001a346 00000000 WebKit!WebCore::MainResourceLoader::addData+0x23 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\mainresourceloader.cpp @ 192] 0031f068 6fdb0b08 05110048 0001a346 0001a346 WebKit!WebCore::ResourceLoader::didReceiveData+0x25 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\resourceloader.cpp @ 276] 0031f184 6fc54d30 05110048 0001a346 0001a346 WebKit!WebCore::MainResourceLoader::didReceiveData+0x188 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\mainresourceloader.cpp @ 512] 0031f1b4 6f8e1a73 7e15d2d0 05110048 0001a346 WebKit!WebCore::ResourceLoader::didReceiveData+0x60 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\resourceloader.cpp @ 430] 0031f1d8 72d46581 03e03a30 03e03c38 0001a346 WebKit!WebCore::didReceiveData+0x43 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\network\cf\resourcehandlecfnet.cpp @ 265] WARNING: Stack unwind information not available. Following frames may be wrong. 0031f2f4 72d48f20 0040aa20 0031f398 003ce8c8 CFNetwork!CFReadStreamCreateWithFormArray+0x6671 0031f480 72d492e5 0031f53c 00000004 03e98b68 CFNetwork!CFReadStreamCreateWithFormArray+0x9010 0031f60c 72d426e2 03e98b9c 00000004 03e03a30 CFNetwork!CFReadStreamCreateWithFormArray+0x93d5 0031f67c 72d438e4 03e9aa80 000004cf 6f7c3956 CFNetwork!CFReadStreamCreateWithFormArray+0x27d2 0031f6a0 75e362fa 018103d6 000004cf 03e03a30 CFNetwork!CFReadStreamCreateWithFormArray+0x39d4 0031f6cc 75e36d3a 72d43860 018103d6 000004cf USER32!InternalCallWinProc+0x23 0031f744 75e377c4 00000000 72d43860 018103d6 USER32!UserCallWinProcCheckWow+0x109 0031f7a4 75e3788a 72d43860 00000000 0031f7e8 USER32!DispatchMessageWorker+0x3bc 0031f7b4 6f7c3611 0031f7cc 0031f830 00000000 USER32!DispatchMessageW+0xf 0031f7e8 6f760eee 76051222 7ee90488 7ee913c0 WebKit!WebCore::RunLoop::run+0x41 [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\platform\win\runloopwin.cpp @ 76] 0031f7fc 6f736766 0031f830 00000000 7ee951f0 WebKit!WebKit::WebProcessMain+0xde [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\win\webprocessmainwin.cpp @ 84] 0031f81c 6f73680c 00000000 00800000 005a14ce WebKit!WebKitMain+0x116 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 59] 0031f848 00801098 00800000 00000000 005a14ce WebKit!WebKitMain+0x9c [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\webprocess\webkitmain.cpp @ 187] 0031fa78 00801258 00800000 00000000 005a14ce WebKit2WebProcess!wWinMain+0x98 [c:\cygwin\home\buildbot\slave\win-release\build\source\webkit2\win\mainwin.cpp @ 67] 0031fb0c 7605339a 7efde000 0031fb58 77d89ef2 WebKit2WebProcess!__tmainCRTStartup+0x150 [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 589] 0031fb18 77d89ef2 7efde000 0cf64ed5 00000000 kernel32!BaseThreadInitThunk+0xe 0031fb58 77d89ec5 008013c4 7efde000 00000000 ntdll!__RtlUserThreadStart+0x70 0031fb70 00000000 008013c4 7efde000 00000000 ntdll!_RtlUserThreadStart+0x1b If I don't hear from you or darin soon, I will have to roll it out :(
mitz
Comment 8
2012-05-25 16:11:51 PDT
(In reply to
comment #7
)
> (In reply to
comment #6
) > > Fixed in <
http://trac.webkit.org/r118568
>. > > I think this caused 19+ crashes to start happening on the Win WK2 Release bots:
This is happening because that build configuration doesn’t enable COMPARE_AND_SWAP.
mitz
Comment 9
2012-05-25 16:17:27 PDT
Since COMPARE_AND_SWAP is an optional feature, we can’t require it for any use of character break iterators. We can either make NonSharedCharacterBreakIterator silently unsafe when COMPARE_AND_SWAP is not enabled, or try to write an alternative, safe implementation that doesn’t rely on COMPARE_AND_SWAP.
mitz
Comment 10
2012-05-25 16:38:47 PDT
Created
attachment 144168
[details]
Added a code path for when COMPARE_AND_SWAP is not enabled
mitz
Comment 11
2012-05-25 16:47:06 PDT
Follow-up patch landed in <
http://trac.webkit.org/r118587
>.
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