The file Tools/TestWebKitAPI/CMakeLists.txt the way it's today makes impossible to other ports to compile some tests on some conditions. As an example, the Nix port need to compile all WK2 API tests but can't use the default TestsController class because it makes a call to WTF::initializeThreads(), a necessary call for WebCore and WTF tests, however such call besides being a layer violation for WK2 tests on Nix point of view it also crash Nix because WTF is compiled as a static library and if we also link the test to WTF the result will be two WTF libraries in the same process leading to a crash. The patch tries to solve that in the way it is already made on other places: define variables storing a list of sources and libraries and let PlatformPORT.cmake overwrite them on their own way.
Created attachment 202732 [details] Patch
Comment on attachment 202732 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202732&action=review I don't have anything against the cosmetics part of the refactoring, but I'm still curious about the paragraph about the crashes in the ChangeLog. Do you still have access do any backtrace? Would the EFL port crash the same way if it were built with SHARED_CORE=OFF? I do remember some problems with duplicated static libraries, but the whole issue is quite tricky so more details would be appreciated. > Tools/ChangeLog:26 > + https://github.com/WebKitNix/webkitnix/commit/f6b80e18cc85e6d7f1e8fdb2ef73ff7ca5d8d3cfgit Nit: this URL leads me to a 404 error page.
The correct URL is: https://github.com/WebKitNix/webkitnix/commit/f6b80e18cc85e6d7f1e8fdb2ef73ff7ca5d8d3cf I'll compile EFL with SHARED_CORE=OFF and post a stacktrace, that should be the same of Nix using the default TestController.
On EFL trunk (r151222), using SHARED_CORE=0, two WK2 tests segfault, the same tests pass on nix, here is the backtrace: /test_webkit2_api_WKStringJSString #0 0x0000000000413b11 in WTF::fastFree(void*) () #1 0x0000000000448f15 in JSStringRelease () #2 0x00000000004074c6 in TestWebKitAPI::WebKit2_WKStringJSString_Test::TestBody() () #3 0x00007ffff54d81aa in testing::Test::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #4 0x00007ffff54d8298 in testing::internal::TestInfoImpl::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #5 0x00007ffff54d832d in testing::TestCase::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #6 0x00007ffff54d85b8 in testing::internal::UnitTestImpl::RunAllTests() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #7 0x0000000000407742 in TestWebKitAPI::TestsController::run(int, char**) () #8 0x0000000000406b29 in main () ./test_webkit2_api_PreventEmptyUserAgent #0 0x0000000000416a41 in WTF::fastFree(void*) () #1 0x000000000046136b in JSC::MarkedBlock::FreeList JSC::MarkedBlock::sweepHelper<(JSC::MarkedBlock::DestructorType)1>(JSC::MarkedBlock::SweepMode) () #2 0x00000000004618ea in JSC::MarkedBlock::sweep(JSC::MarkedBlock::SweepMode) () #3 0x0000000000462d4c in JSC::MarkedSpace::lastChanceToFinalize() () #4 0x00000000004d7ed5 in JSC::VM::~VM() () #5 0x00000000004eea48 in JSC::JSLockHolder::~JSLockHolder() () #6 0x000000000044c87b in JSGlobalContextRelease () #7 0x0000000000409eff in TestWebKitAPI::didRunJavaScript(OpaqueWKSerializedScriptValue const*, OpaqueWKError const*, void*) () #8 0x00007ffff5c0b44c in WebKit::WebPageProxy::scriptValueCallback(CoreIPC::DataReference const&, unsigned long) () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #9 0x00007ffff5d2505a in WebKit::WebPageProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&) () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #10 0x00007ffff5b8189b in CoreIPC::MessageReceiverMap::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&) () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #11 0x00007ffff5c18202 in WebKit::WebProcessProxy::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&) () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #12 0x00007ffff5b7bd65 in CoreIPC::Connection::dispatchMessage(WTF::PassOwnPtr<CoreIPC::MessageDecoder>) () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #13 0x00007ffff5b7beb3 in CoreIPC::Connection::dispatchOneMessage() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #14 0x00007ffff61a6a83 in WebCore::RunLoop::performWork() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libewebkit2.so.0 #15 0x00007ffff3e291eb in _ecore_pipe_read (data=0xb0e520, fd_handler=<optimized out>) at ecore_pipe.c:632 #16 0x00007ffff3e28271 in _ecore_call_fd_cb (fd_handler=0xb0ead0, data=<optimized out>, func=<optimized out>) at ecore_private.h:345 #17 _ecore_main_fd_handlers_call () at ecore_main.c:1670 #18 _ecore_main_loop_iterate_internal (once_only=once_only@entry=1) at ecore_main.c:1917 #19 0x00007ffff3e28625 in ecore_main_loop_iterate () at ecore_main.c:889 #20 0x000000000040a265 in TestWebKitAPI::Util::run(bool*) () #21 0x0000000000408861 in TestWebKitAPI::WebKit2_PreventEmptyUserAgent_Test::TestBody() () #22 0x00007ffff54d81aa in testing::Test::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #23 0x00007ffff54d8298 in testing::internal::TestInfoImpl::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #24 0x00007ffff54d832d in testing::TestCase::Run() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #25 0x00007ffff54d85b8 in testing::internal::UnitTestImpl::RunAllTests() () from /home/hugo/src/webkit/WebKitBuild/Release/lib/libgtest.so #26 0x000000000040a672 in TestWebKitAPI::TestsController::run(int, char**) () #27 0x0000000000408599 in main () These problems on WTF::fastFree could be caused by something fastAlloced by in one "library space" and fastFreed by another one, e.g. allocated in WK2 library and freed by the unit test, that have a copy of some WTF pieces, further investigation is needed but the whole point is: There's no sense in a WK2 test link against JSC, WebCore or WTF libraries if we are just testing the WK2 API layer.
(In reply to comment #4) > These problems on WTF::fastFree could be caused by something fastAlloced by in one "library space" and fastFreed by another one, e.g. allocated in WK2 library and freed by the unit test, that have a copy of some WTF pieces, further investigation is needed I didn't get these errors here, and they shouldn't happen since <http://trac.webkit.org/changeset/147743>, which disables GLOBAL_FAST_MALLOC_NEW. I did get a crash in test_webkit2_api_PreventEmptyUserAgent which is related to the libraries which get linked to the unit tests, though, and it looks remarkably similar to the crash I normally get when I run layout tests without SHARED_CORE=ON. > but the whole point is: There's no sense in a WK2 test link against JSC, WebCore or WTF libraries if we are just testing the WK2 API layer. Actually, this is more about the linking order than what gets linked -- even if you pass only "WebKit2" to TARGET_LINK_LIBRARIES(), it will pull in its dependencies (ie. WTF, JSC etc), but everything will be linked in the correct order (ie. shared library first, static libraries later). If you change test_webkit2_api_LIBRARIES and pass WebKit2 before WTF and JSC, the crashes should also go away. Can you confirm if that happens for you as well?
(In reply to comment #5) > (In reply to comment #4) > > These problems on WTF::fastFree could be caused by something fastAlloced by in one "library space" and fastFreed by another one, e.g. allocated in WK2 library and freed by the unit test, that have a copy of some WTF pieces, further investigation is needed > > I didn't get these errors here, and they shouldn't happen since <http://trac.webkit.org/changeset/147743>, which disables GLOBAL_FAST_MALLOC_NEW. IIRC GLOBAL_FAST_MALLOC_NEW just disable the global overloads, but the WTF classes (Hash, String, etc) continue to use then internally. > Actually, this is more about the linking order than what gets linked -- even if you pass only "WebKit2" to TARGET_LINK_LIBRARIES(), it will pull in its dependencies (ie. WTF, JSC etc), but everything will be linked in the correct order (ie. shared library first, static libraries later). > > If you change test_webkit2_api_LIBRARIES and pass WebKit2 before WTF and JSC, the crashes should also go away. Can you confirm if that happens for you as well? Ok, I'll test it.
(In reply to comment #5) > If you change test_webkit2_api_LIBRARIES and pass WebKit2 before WTF and JSC, the crashes should also go away. Can you confirm if that happens for you as well? Same error here after the change :-/, btw I think this patch has nothing to do with this issue, the patch just open the possibility to other ports define the to be used to link with WK2 tests.
Comment on attachment 202732 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202732&action=review > Tools/TestWebKitAPI/PlatformEfl.cmake:59 > +set(webkit2TestList what about moving this variable into CMakeLists.txt and then do a list(REMOVE) for the failing/not-implemented tests in the PlatformXXX files?
(In reply to comment #8) > (From update of attachment 202732 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202732&action=review > > > Tools/TestWebKitAPI/PlatformEfl.cmake:59 > > +set(webkit2TestList > > what about moving this variable into CMakeLists.txt and then do a list(REMOVE) for the failing/not-implemented tests in the PlatformXXX files? Seems a nice idea to me, but this implies removing the "webkit2FailTestList" variable and not compiling failing tests, right? I'm ok with that, if the test will fail for sure there's no reason to compile it but I would like to know what others think about it, Rakuko?
Comment on attachment 202732 [details] Patch Cleared review? from attachment 202732 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).