621527 - ShadowLayersParent::RecvUpdate crashes when trying to use destroyed layer manager

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Tatiana Meshkova (:tatiana)]

Attachment: [@SetAllocator].bt

Created attachment 498983 [details]
[@cairo_surface_reference].bt

STEPS LEADING TO PROBLEM: 
1. Open Fennec and navigate to www.lemonde.fr
2. Click on top page banner (it opens new tab)
3. Close Fennec as fast as you can

EXPECTED OUTCOME:
no crash observed

ACTUAL OUTCOME:
TaskSwitcherWindow::closeEvent 
TaskSwitcherWindow::closeTab 
TaskSwitcherWindow::notifyObservers aBrowser: 0xa19270, aTopic: fennec-taskswitcher-window-closed
TaskSwitcherWindow::notifyObservers Notifying observers with topic 'fennec-taskswitcher-window-closed'
virtual nsresult nsWindow::Destroy():402
virtual void mozilla::layers::LayerManagerOGL::Destroy():93
TaskSwitcherWindow::~TaskSwitcherWindow 
layer_manager()->IsDestroyed(): 1
[ParentSide] CreateImageLayer

###!!! [Child][SyncChannel] Error: Channel error: cannot send/recv

Segmentation fault (core dumped)

Comment 1

[Tatiana Meshkova (:tatiana)]

Attachment: patch: ignore update if layer manager IsDestroyed()

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Is this crash happening before or after the PBrowser:Destroy message is received by the content process?  (At TabChild::RecvDestroy.)

Comment 3

[Tatiana Meshkova (:tatiana)]

(In reply to comment #2)
> Is this crash happening before or after the PBrowser:Destroy message is
> received by the content process?  (At TabChild::RecvDestroy.)

before

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 499836 [details] [diff] [review]
patch: ignore update if layer manager IsDestroyed()

Thanks.  This is a somewhat ugly race condition.  We eventually should propagate the destroyed state discovered during this transaction back to the child process, and do an analogous Destroy() there, but AFAIK this patch should be fine for now, because of the way shadow OGL layers work.  Can look into the harder bits with a followup.

Comment 5

[Oleg Romashin (:romaxa)]

I guess it should be safe to push

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 499836 [details] [diff] [review]
patch: ignore update if layer manager IsDestroyed()

This crash is also very easy to hit in reftest-ipc+GL.

Can we pretty please get blocking+ or approval?

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/959744a69f80