| Summary: | [BlackBerry] destruction sequence in LayerCompositingThread is confusing | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joe Mason <joenotcharles> | ||||
| Component: | WebKit BlackBerry | Assignee: | Joe Mason <joenotcharles> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Minor | CC: | anilsson, levin+threading, tonikitoo, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Other | ||||||
| OS: | Other | ||||||
| Attachments: |
|
||||||
Created attachment 134114 [details]
patch
Comment on attachment 134114 [details]
patch
Looks good.
Comment on attachment 134114 [details] patch Clearing flags on attachment: 134114 Committed r112304: <http://trac.webkit.org/changeset/112304> All reviewed patches have been landed. Closing bug. |
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.)