Bug 81706 - [BlackBerry] destruction sequence in LayerCompositingThread is confusing
Summary: [BlackBerry] destruction sequence in LayerCompositingThread is confusing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Minor
Assignee: Joe Mason
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-20 15:46 PDT by Joe Mason
Modified: 2012-03-27 13:06 PDT (History)
4 users (show)

See Also:


Attachments
patch (5.39 KB, patch)
2012-03-27 12:17 PDT, Joe Mason
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joe Mason 2012-03-20 15:46:10 PDT
LayerCompositingThread's destructor looks like this:

LayerCompositingThread::~LayerCompositingThread()
{
    // Unfortunately, ThreadSafeShared<T> is hardwired to call T::~T().
    // To switch threads in case the last reference is released on the
    // WebKit thread, we send a sync message to the compositing thread.
    destroyOnCompositingThread();
}

And then destroyOnCompositingThread does a synchronous dispatch to the compositing thread to clean up the object.  It's weird to have a dispatch like this happen from within the destructor, and it's easy to miss the fact that it's synchronous and believe this is a delayed destruction.

It would be much cleaner to flip this around: have ThreadSafeShared<T> call a function called destroyOnCompositingThread, which calls "delete this", and move all the cleanup into the standard destructor.  (Also, this might let us get rid of the sync dispatch and just use a regular dispatch.)
Comment 1 Joe Mason 2012-03-27 12:17:44 PDT
Created attachment 134114 [details]
patch
Comment 2 Rob Buis 2012-03-27 12:21:03 PDT
Comment on attachment 134114 [details]
patch

Looks good.
Comment 3 WebKit Review Bot 2012-03-27 13:06:30 PDT
Comment on attachment 134114 [details]
patch

Clearing flags on attachment: 134114

Committed r112304: <http://trac.webkit.org/changeset/112304>
Comment 4 WebKit Review Bot 2012-03-27 13:06:34 PDT
All reviewed patches have been landed.  Closing bug.