WebPositive: leak of WorkQueue (CryptoQueue, ImageDecoder, etc.) threads

[X512]

This is Haiku 54101.

A lot of "CryptoQueue" threads are running in WebPositive even if all tabs are closed.

[waddlesplash]

Is this still a problem under unreleased WebKit? Can you get a backtrace of a few of these threads?

[X512]

Stack trace of CryptoQueue thread

[X512]

In latest WebKit CryptoQueue threads are still present if all tabs are closed, but there are no so many of them. Maybe longer browsing session is required to collect more threads.

[waddlesplash]

They appear to just be waiting for messages; so something is not posting a QUIT message where it should.

[pulkomandy]

It's a "work queue" thread created here: ​https://github.com/WebKit/webkit/blob/fffc493a0bb0ea8ef3beb44d9bc2892d0eeeaf5e/Source/WebCore/crypto/SubtleCrypto.cpp

So probably our work queue implementation isn't correct. The crypto code is shared with other platforms.

[waddlesplash]

The same happens to ImageDecoder, there are eventually hundreds of such threads. It appears these are also WorkQueues (see ImageSource.cpp.)

[nephele]

I can't reproduce this issue, are there some simple instructions?

[pulkomandy]

Nothing special, just browse the web for a while (probably websites that use webcrypto in some way?) and the threads will pile up.

[nephele]

waddlesplash linked me to one example site that makes one thread leak, ​https://html5test.com. I don't think instructions of the sort of "just browse" are very usefull, it's not easy to reproduce like that, especially with a rare issue like this, even if it is more common it would be nice to include atleast one example for how to reproduce an issue.

[pulkomandy]

Yes, but no one had investigated it yet to find a site that triggers this easily. That is part of the work in fixing these problems.

[waddlesplash]

Google Maps triggers this pretty badly, after a few minutes there will be quite literally hundreds of threads.

[nephele]

Please retest with ​https://git.gryphno.de/nephele/haikuwebkit/commit/9c60ce5494c83469bdb03a2c02f817d149f19134

[waddlesplash]

Fix committed in ​https://github.com/haiku/haikuwebkit/commit/9a2ba62671e24f6967f9932b38d49112f06be78d

[nephele]

why the beta4 milestone?

[waddlesplash]

Because it was fixed by/for beta4. It may appear in the nightlies before that, or even in a beta3 backport, though.

[nephele]

haikuwebkit is not part of specific versions of haiku, it will appear in the next regular release for beta3 afaik. I don't think it is correct to say it was fixed for beta4 since we use the milestones as an estimate of fixed bugs in releases

[pulkomandy]

It is broken in beta3 (even if the fix will be available as an update there. It will be working in beta4. Therefore it is fixed somewhere between beta3 and beta4.

So the beta4 milestone is correct.

Also note that this is a milestone, not a "fixed in" or something like that.

I don't want to be managing separate milestones or versions for WebKit here. So let's just use the Haiku ones.

[nephele]

I don't want to be managing separate milestones or versions for WebKit here. So let's just use the Haiku ones.

Okay, I would have assumed to use "no milestone" instead, I had closed tickets like that before, I shall use the haiku milestones then.

[waddlesplash]

Unfortunately this still appears to be a problem on 1.8.3 despite the fix: there are still lots of ImageDecoder threads that rapidly accumulate when browsing Google Maps.

[leavengood]

Surely we should actually have a thread pool here and not spin up a new thread for every task? Though maybe that is the source of the bug here. I plan to start looking at our WebKit now though I am very out-of-date and obviously it is a huge project.

[pulkomandy]

As far as I know this is all in shared WebKit code and we don't really have control on it. They probably already use a kind of thread pool, and start new threads only if all running ones are already busy.

But there is possibly something wrong with our implementation of the thread class that goes under that and is implemented using spawn_thread and other BeOS APIs. Maybe the threads fail to report that they are idle, or fail to stop when asked to do so.

[pulkomandy]

Related WebKit bug: ​https://bugs.webkit.org/show_bug.cgi?id=181711

Are there still problems with CryptoQueue threads too?

[pulkomandy]

I made some more investigations yesterday, and adjusted the patch from that ticket to build with current versions of WebKit. Now instead of a high number of threads, I get only 4, but then they stop after decoding a small number of images. It works better if I add printf statements in some places.

I think the RunLoop implementation for Haiku is not correct. We can try making it closer to RunLoopWindows which is implemented in a somewhat similar way.

There is a RunLoopGeneric but we can't use it for the main runloop that must run in the thread of BApplication so it has to be a BHandler. So we have to implement our own.

[pulkomandy]

Fixed in ​https://github.com/haiku/haikuwebkit/commit/6e6c7e698f6f4dca715ff7ef539b944a19614f33