[webkit-dev] Issues with WebWorkers that may block Safari support

[Maciej Stachowiak]

On Dec 4, 2009, at 2:09 PM, Drew Wilson wrote:

>
>
> On Fri, Dec 4, 2009 at 2:00 PM, Maciej Stachowiak <mjs at apple.com>  
> wrote:
>
> On Dec 4, 2009, at 1:40 PM, Drew Wilson wrote:
>
>> Hi all,
>>
>> There have been various side discussions regarding the stability/ 
>> viability of Workers and SharedWorkers, and I wanted to make sure  
>> we understand all of the concerns, since as far as I know there  
>> isn't any one person who has been privy to all of the conversations  
>> (or maybe it's just me that's been out of the loop :). So I'll  
>> enumerate the issues I'm aware of below, and people can chime in on  
>> any that I've missed that may impact the ability to ship these  
>> features, and we can figure out how to get them addressed ASAP.
>>
>> 1) worker-lifecycle.html fails intermittently (https://bugs.webkit.org/show_bug.cgi?id=29344 
>> )
>>
>> The original report doesn't seem to be a true worker failure, but  
>> rather an artifact of the way GC works in JSC. Since JSC uses  
>> conservative GC, it's quite possible for the VM to think that  
>> there's a dangling reference to a Worker even though there isn't  
>> actually one, and that seems to be what's happening here (workers  
>> would get shutdown when the parent document closes regardless, so  
>> there's no actual leak). Unless someone has some idea of how to  
>> make this GC more deterministic, my recommendation would be to just  
>> disable this test and close this bug, since seemingly it's the test  
>> itself that is not reliable, not the underlying worker code.
>>
>> That said, there's a sporadic worker crash that seems to have  
>> started happening in the last week or so which I believe is a  
>> different issue - eseidel has glommed that crash onto the same bug,  
>> and I think we should move that to its own issue and see if we can  
>> collect more information about it.
>
> For what it's worth, we'd likely consider a resolution to this bug a  
> blocker to shipping the next Safari release, but we would not  
> consider disabling dedicated Workers as a solution, since we have  
> shipped them already.
>
> If this bug could be shown to be an error in the test and  
> inappropriate dependence on GC, that would be a satisfactory  
> resolution. However, the sporadic failure here is a at least  
> sometimes a crash. A crash definitely indicates a bug in WebKit, not  
> in the test, even if the test itself is also flawed. Therefore, I  
> would not consider disabling the test a solution until we have at  
> least addressed the crash.
>
> That makes sense to me - we should try to address the crash, and if  
> keeping this test enabled is the only way to do that, that's fine. I  
> would recommend changing the test, however, so the test itself  
> always passes so it's not making the bots red while we're waiting to  
> reproduce the crash.

As long as we have some bug report tracking the crash and add a new  
test when we fix it that would detect it, I'm fine with that.

> Regarding Geoff's comments - I'm as convinced as I can be (based on  
> my own and ap's efforts stepping through the GC code - see ap's  
> comments in the bug) that the original failures were a "conservative  
> GC" issue. I'm honestly stumped as to how to move forward on that.

We should definitely change the test to not depend on that, if we can.  
I am a little dubious of the sporadic failure truly being a  
conservative GC issue and not being fixable; in the past we have  
managed to make GC-related tests reliable even if they could be  
affected by conservative GC in theory. I'll look at the test myself  
and see if any changes seem like they would help.

But as I said, I'm more concerned with the crash than the sporadic non- 
crash failure.

>
>>
>> 3) Potential race conditions with Document.postTask() (https://bugs.webkit.org/show_bug.cgi?id=31633 
>> )
>>
>> I've suggested a simple solution to the potential race condition  
>> with SharedWorkers in the bug. I'd be interested in hearing whether  
>> people think it's a good approach or not - if so, I'm happy to  
>> submit a patch for review in short order. There's still the  
>> question as to whether dispatching tasks on a detached Document is  
>> OK or not, but I'm assuming it is (since that's what Dedicated  
>> workers have always done).
>>
>> Are there other issues we should be addressing as a high priority?
>
> This one would probably be a ship blocker. We would consider  
> disabling SharedWorkers to ship if that addressed the issue.
>
> Any objections to the solution I proposed?

Haven't had a chance to study it yet, but I'll comment in the bug if I  
have any thoughts. I'm not an expert in this code though so I'm not  
sure my own input is the most useful here.

Regards,
Maciej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091204/65034506/attachment.html>