Bug 38502 – [Qt] Qt build system failed to notice dependency changes after r58712

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Dmitry Titov]

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.

[Eric Seidel (no email)]

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.

[Gavin Barraclough]

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?

[Eric Seidel (no email)]

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?

[Eric Seidel (no email)]

Thank you both for your prompt followup.

[Eric Seidel (no email)]

I always hit the security checkbox by accident. :(

[Gavin Barraclough]

(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. :-)

[Csaba Osztrogonác]

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.

[Dmitry Titov]

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

[Dmitry Titov]

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.

[Gavin Barraclough]

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.

[Gavin Barraclough]

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

[Gavin Barraclough]

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.

[Benjamin Poulain]

Uh, shouldn't this be closed?

[Csaba Osztrogonác]

(In reply to comment #13)
> Uh, shouldn't this be closed?

Yes.