Bug 65685 – [chromium] Fix assertion failure in ThreadIdentifierData::identifier in TestShell.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

Fix assertion failure in ThreadIdentifierData::identifier for non-WTF managed threads

Created attachment 102904 [details]
Patch

This assertion fails when I run DumpRenderTree to generate extension docs in chrome, e.g.

~/src/chrome/common/extensions/docs/build$ ./build.py --nozip --page-name experimental.webRequest
ASSERTION FAILED: m_key != PTHREAD_KEYS_MAX
third_party/WebKit/Source/JavaScriptCore/wtf/ThreadIdentifierDataPthreads.cpp(60) : static WTF::ThreadIdentifier WTF::ThreadIdentifierData::identifier()
[3445:3445:715988328322:ERROR:process_util_posix.cc(134)] Received signal 11
	base::debug::StackTrace::StackTrace() [0x7fceb65120c6]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x7fceb6568db1]
	0x7fceaff1baf0
	WTF::ThreadIdentifierData::identifier() [0x538e88]
	WTF::currentThread() [0x51f818]
	WTF::ThreadRestrictionVerifier::setShared() [0x4cda7a]
	WTF::RefCountedBase::ref() [0x4d4768]
	WTF::refIfNotNull<>() [0x4d4fb7]
	WTF::RefPtr<>::operator=() [0x4d4ba2]
	TestNavigationController::loadEntry() [0x4d3efd]
	WebViewHost::loadURLForFrame() [0x4e6d71]
	TestShell::loadURL() [0x4d7037]
	TestShell::runFileTest() [0x4d6d7c]
	runTest() [0x4ac71c]
	main [0x4ad1f0]
	0x7fceaff06c4d
	0x49b749
Traceback (most recent call last):
  File "./build.py", line 245, in <module>
    sys.exit(main())
  File "./build.py", line 219, in main
    modified_files = RenderPages(page_names, dump_render_tree)
  File "./build.py", line 89, in RenderPages
    (dump_render_tree, output))
Exception: /home/eisinger/chrome/trunk/src/out/Debug/DumpRenderTree returned invalid output:

#URL:file:///home/eisinger/chrome/trunk/src/chrome/common/extensions/docs/build/generator.html?experimental.webRequest

[WebKit Review Bot]

Comment on attachment 102904 [details]
Patch

Attachment 102904 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9315410

New failing tests:
canvas/philip/tests/2d.text-custom-font-load-crash.html
http/tests/appcache/different-origin-manifest.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-controller-drt-api.html
canvas/philip/tests/2d.pattern.modify.image2.html
http/tests/appcache/document-write-html-element.html
canvas/philip/tests/2d.pattern.animated.gif.html
animations/animation-drt-api.html
animations/animation-end-event-destroy-renderer.html
animations/animation-drt-api-multiple-keyframes.html
http/tests/appcache/destroyed-frame.html
canvas/philip/tests/2d.pattern.modify.image1.html
animations/animation-direction.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
canvas/philip/tests/2d.drawImage.animated.apng.html
http/tests/appcache/fail-on-update-2.html
http/tests/appcache/cyrillic-uri.html
canvas/philip/tests/2d.drawImage.animated.gif.html
animations/animation-direction-normal.html

[WebKit Review Bot]

Created attachment 102907 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 102904 [details]
Patch

Attachment 102904 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9300659

Comment on attachment 102904 [details]
Patch

obviously that didn't solve the problem either. No idea why this isn't working for me :-/

Apparently, pthread_create_key is invoked and returns 0, but the key isn't set

btw, it's quite hacky to set m_key to PTHREAD_KEYS_MAX as pthread_key_t isn't guaranteed to by an integer

[David Levin]

This is the wrong fix. The real problem is that wtf is being used without having been initialized at all in the process.

At a minimum, one needs to call WTF::initializeThreading before using wtf (http://www.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/JavaScriptCore/wtf/ThreadingPthreads.cpp&l=78).

For chromium code, WebKit::initialize (Chromium public api) should be called before calling the webkit api (http://www.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebKit/chromium/src/WebKit.cpp&l=69). Note that not calling WebKit::initialize may also result in issues if the code uses javascript because various bits of v8 (e.g. isolates) are initialized there.

[David Levin]

(In reply to comment #6)
> btw, it's quite hacky to set m_key to PTHREAD_KEYS_MAX as pthread_key_t isn't guaranteed to by an integer

fwiw, this value is only used for the assert to catch bug which it appears to be doing quite well, so as long as we aren't getting false positives, it seems fine. 

oth, we shouldn't (and don't) rely on that value at all in production code.

[David Levin]

Created attachment 102978 [details]
Patch

[Dmitry Titov]

Comment on attachment 102978 [details]
Patch

It would be helpful if you add a note (to ChangeLog preferably) about why initialization is done in this particular place.

[David Levin]

Created attachment 102984 [details]
Patch

[WebKit Review Bot]

Comment on attachment 102984 [details]
Patch

Clearing flags on attachment: 102984

Committed r92420: <http://trac.webkit.org/changeset/92420>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.