Bug 38502

Summary: [Qt] Qt build system failed to notice dependency changes after r58712
Product: WebKit Reporter: Dmitry Titov <dimich>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, eric, ossy
Priority: P2 Keywords: LayoutTestFailure, Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   

Description Dmitry Titov 2010-05-03 18:40:12 PDT
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11163
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11168
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11169

It started happening with r58712 although it's possible this change shifted timing enough for some existing race to get worse.

Unfortunately, it seems the release Qt builds do not have symbols.
Comment 1 Eric Seidel (no email) 2010-05-03 19:49:00 PDT
Either way, we can't leave this crashing.  We need to revert the revert or skip the test or fix whatever is causing this.  We can't just leave the tree red.
Comment 2 Gavin Barraclough 2010-05-03 20:47:40 PDT
I believe that Ossy reported that this was caused by the QT build script failing to detect a dependency change after the update pulled in r58712, and that a clean build fixed the problem for him in his local build.

Did I misunderstand something, or did that turn out not to be the case?
Does this just require a nice clean build on the bot?, or is there another problem?
Comment 3 Eric Seidel (no email) 2010-05-03 20:57:18 PDT
You're right, this has resolved itself.  So I guess we should change this bug to be a bug in the Qt build system then.  Do we know what about the dependencies the build script failed to notice?
Comment 4 Eric Seidel (no email) 2010-05-03 20:57:57 PDT
Thank you both for your prompt followup.
Comment 5 Eric Seidel (no email) 2010-05-03 21:00:02 PDT
I always hit the security checkbox by accident. :(
Comment 6 Gavin Barraclough 2010-05-03 22:15:45 PDT
(In reply to comment #3)
> Do we know what about the
> dependencies the build script failed to notice?

I believe Ossy thought he understood this, he explained to me on irc something like:

(1) The revert removed wtf/text/AtomicStringTable.h
(2) ...
(3) This causing some .o's to not get rebuilt.

I'm afraid I didn't really follow step 2 myself, but Ossy seemed to see what was going on. :o)

As a workaround I suggested we could make the QT bot spot files being removed in an update & force a rebuild.  Ossy appeared to think this might work, but was going to try to fix the dependencies in the makefile properly first, before resorting to hackery. :-)
Comment 7 Csaba Osztrogonác 2010-05-04 02:47:57 PDT
r58712 made fast/workers/worker-gc2.html crash. I played a lot with this bug.
If I skipped fast/workers/worker-gc2.html, fast/workers/worker-gc.html crashed. And I had a suspicion, I thought it is a DRT sideeffect. After skipped the test before fast/workers/worker-gc.html, I didn't get any crash. (with run-webkit-tests fast/workers --iterations 100)

So I skipped fast/workers/worker-event-listener.html until fix:
http://trac.webkit.org/changeset/58746

The other potential problem, dependency feature is unrelated to
this crash, but I'm going to examine it too.
Comment 8 Dmitry Titov 2010-05-04 10:47:09 PDT
Yep, unfortunately, this wasn't the issue of clean build and we still have a failure here, Ossy had to disable a test to green the tree. Here is a sequence of events:

The first this crashed after r58712 even though suspicion is that change only changed timing to expose existing issue:
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11158 

Ossy triggered clean build on the bot, which helped, lathough temporarily (perhaps shifting :
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11164 

After some more builds (and perhaps a bit of change in timing again) the crash re-appeared:
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11181

Ossy finds out the test fails because of the sideeffect of the preceeding worker-event-listener.html and disables that on Qt, bot goes green again:
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/11194
Comment 9 Dmitry Titov 2010-05-04 11:08:37 PDT
Discussed with Ossy, we need to split this issue in 2 - about failing test and dependency issue in build system.

More details on dependency issue, compiled from IRC chat:

When you remove a file from the tree, it will be removed from the makefiles, dependencies, etc. But if you remove the dependency, it might be some trash in an object, lib. Try to imagine a foo.so : a.o b.o c.o rule, and if you remove a.c from the tree, the new rule will be foo.so : b.o c.o , without a.o. This rule won't trigger a new linking.
Comment 10 Gavin Barraclough 2010-05-05 13:20:32 PDT
Okay, I think I've figured out the worker-gc2 crash, and I think it's related to a change I made a few weeks ago.

The bug was in existence prior to r58114.  This introduced leaks, which will have masked the bug (the bug is a use after release of the objects leaked by r58114).  r58712 then appeared to break the test, since it stopped leaking these objects again, making the bug possible.

The bug is currently only biting on QT since it is platform dependent – it depends on the ordering that ThreadSpecifics are deleted, which is not defined.

patch incoming.
Comment 11 Gavin Barraclough 2010-05-05 13:30:46 PDT
Actually, I shouldn't steal this bug – is there still an issue to resolve re the build system's dependencies Ossy?

new bug: https://bugs.webkit.org/show_bug.cgi?id=38604
Comment 12 Gavin Barraclough 2010-05-05 18:14:10 PDT
bug #38604 is fixed in r58851, hopefully this should be the issue that was hitting fast/workers/worker-gc2.html on Qt.  With a bit of luck there's now just the build dependency problem to resolve.
Comment 13 Benjamin Poulain 2011-01-30 08:30:14 PST
Uh, shouldn't this be closed?
Comment 14 Csaba Osztrogonác 2012-02-02 10:26:22 PST
(In reply to comment #13)
> Uh, shouldn't this be closed?

Yes.