[Webkit-unassigned] [Bug 34014] WebSocket wrapper can be collected even if events are pending

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34014





--- Comment #5 from Fumitoshi Ukai <ukai at chromium.org>  2010-01-31 17:43:38 PST ---
(In reply to comment #2)
> (From update of attachment 47655 [details])
> +    LOG(Network, "WebSocket %p scriptExecutionContext destroyed", this);
> 
> There may be too many logs long term - very useful when writing the code, but
> perhaps overly noisy when debugging other network-related issues.
> 
> We'll need to review the use of LOG() in WebSocket code as it matures.
> 
> I'd love to see per-function comments - the addition of contextDestroyed()
> really needs an explanation of why it was needed. Actually, is it also a fix
> for bug 33248?

I implemented stop() method to close the connection, and contextDestoryed()
just checks the connection was already closed.
It seems this fixes for bug 33248 as well.  I'll write layout test for it.

> 
> +    if (m_channel)
> +        ActiveDOMObject::unsetPendingActivity(this);
> +    m_channel = 0;
> 
> Accessing "this" after unsetPendingActivity seems dangerous. I don't see how
> this can cause issues with current code, but that's the call that lets the
> object get deleted eventually. I think that it would be good defensive
> programming to move access to m_channel up.

Thanks.  Fixed.

> 
> r=me. Please consider addressing the comments about ChangeLog and m_channel.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.