[Webkit-unassigned] [Bug 237060] Queue throwing exception of blob objects during destruction

[bugzilla-daemon at webkit.org]

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

--- Comment #12 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 452987
  --> https://bugs.webkit.org/attachment.cgi?id=452987
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452987&action=review

>>> Source/WebCore/fileapi/Blob.cpp:292
>>> +                context->eventLoop().queueTask(TaskSource::InternalAsyncTask, [this, protectedThis = Ref { *this }, code] {
>> 
>> The fact that we're needing to ref `this` here feels wrong because we're calling this from the destructor.  Why do we need to move the exception into m_exception if the object is already being destructed, granted, that didFail() may be called from places not in the destruction path.  This hints that this Blob code is itself not aware of what it's doing.  Let's get input from a WebCore engineer on this to determine if this is actually correct to do.
> 
> Blob is an ActiveDOMObject, we should probably call queueTaskKeepingObjectAlive() from ActiveDOMObject base class?
> 
> queueTaskKeepingObjectAlive(*this, TaskSource::InternalAsyncTask, [this, code] {
>   // ...
> });
> 
> Note that you wouldn't need to capture protectedThis then since queueTaskKeepingObjectAlive does this for you (in addition to keeping the JS wrapper alive).

Oh, I missed Mark's comment about this function getting called from the destructor. Protecting |this| here is definitely wrong then. Calling queueTaskKeepingObjectAlive() would be wrong too.

I'll need to see the call stack to try and suggest something better.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220223/483aad69/attachment-0001.htm>