[B2G/SkiaGL] With SurfaceStreamType::SingleBuffer, canvas app crashed when resumed app to foreground Based on 843599, I modified the SurfaceStreamType as SingleBuffer for 2D canvas. The following shows the crash of canvas app. 01-17 23:32:55.268 2083 2083 I Gecko : CanvasClientWebGL::Update this 0x4317d4e0 buffer 0x4447fa80 TextureClient 0x4319e740 cnt 1 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: could not look up PGrallocBuffer 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: Error deserializing 'bufferParent' (PGrallocBuffer) member of 'SurfaceDescriptorGralloc' 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: Error deserializing 'image' (SurfaceDescriptor) member of 'OpPaintTexture' 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: Error deserializing 'Edit[i]' 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: Error deserializing 'InfallibleTArray' 01-17 23:32:55.268 2001 2027 I Gecko : 01-17 23:32:55.268 2001 2027 I Gecko : ###!!! [Child][SyncChannel] Error: Value error: message was deserialized, but contained an illegal value 01-17 23:32:55.268 2001 2027 I Gecko : 01-17 23:32:55.278 2001 2001 I Gecko : 01-17 23:32:55.278 2001 2001 I Gecko : ###!!! [Parent][AsyncChannel] Error: Channel error: cannot send/recv 01-17 23:32:55.278 2001 2001 I Gecko : 01-17 23:32:55.288 2001 2012 I Gecko : [Parent 2001] WARNING: pipe error (134): Connection reset by peer: file /media/ramdisk/b2g_central_new/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 431 01-17 23:32:55.288 2001 2027 E memalloc: /dev/pmem: Freeing buffer base:0x4a336000 size:573440 offset:618496 fd:107 01-17 23:32:55.288 2001 2027 I Gecko : [Parent 2001] ###!!! ABORT: actor has been |delete|d: file /home/peter/B2G_debug/objdir-rb2g_central_new/ipc/ipdl/PGrallocBufferParent.cpp, line 392 01-17 23:32:55.288 2001 2027 E Gecko : mozalloc_abort: [Parent 2001] ###!!! ABORT: actor has been |delete|d: file /home/peter/B2G_debug/objdir-rb2g_central_new/ipc/ipdl/PGrallocBufferParent.cpp, line 392

Change tile because canvas app crashed with triplebuffer type too.

Found cnavas app crashed at below two cases a. switch to foreground b. two or more canvas layers that share the same GLContext creation/free (Like cut the rope)

Found canvas app crashed at below two cases a. switch to foreground b. two or more canvas layers that share the same GLContext creation/free (Like cut the rope)

Create patch to destroy backsurface for canvas 2D to fix crash issues in comment 3.

01-17 23:32:55.268 2083 2083 I Gecko : CanvasClientWebGL::Update this 0x4317d4e0 buffer 0x4447fa80 TextureClient 0x4319e740 cnt 1 01-17 23:32:55.268 2001 2027 I Gecko : IPDL protocol error: could not look up PGrallocBuffer So in my investigation, one weird thing is the PGrallocBuffer that could not be looked up is a value that looks a lot like a pointer, not an ID. That worries me; we shouldn't be having a pointer value ever escape in there. Even if this fixes it, I'd like to understand where that value is coming from. (In reply to pchang from comment #3) > b. two or more canvas layers that share the same GLContext creation/free > (Like cut the rope) Hm, I don't understand this. More than one canvas layer should never share the same GLContext; how are we getting into this state? CTR (JS) does have more than one canvas (2D) element, and even if they were backed by Skia, they should have different GLContexts right?

I tracked this down. The issue is that the TextureHost goes away (because, I believe, we create a new set of Hosts/Clients for a new set of layers, since we threw away everything when we discarded things we didn't need), and this line: https://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/TextureHost.cpp#81 causes us to explicitly deallocate the GraphicBuffer. Commenting out this line makes things work fine, though we're probably leaking like crazy. I think the fix is, then, to be able to tell TextureHost "you're only borrowing this; don't delete it, we're tracking its lifecycle elsewhere".

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #6) > I tracked this down. The issue is that the TextureHost goes away (because, > I believe, we create a new set of Hosts/Clients for a new set of layers, > since we threw away everything when we discarded things we didn't need), and > this line: > > https://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ > TextureHost.cpp#81 > It is correct. > causes us to explicitly deallocate the GraphicBuffer. Commenting out this > line makes things work fine, though we're probably leaking like crazy. I > think the fix is, then, to be able to tell TextureHost "you're only > borrowing this; don't delete it, we're tracking its lifecycle elsewhere". I made another patch for the "borrowing" concept and the changes were not too much. But this approach uses more memory than "free consumer" version when app switches to background.

create patch for "borrowing" concept

Cool, that's nice and simple. But I see we already have: 35 // The host is responsible for tidying up any shared resources. 36 const TextureFlags HostRelease = 0x20; which is only used here: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClient.cpp#56 We should rationalize these, otherwise we're introducing just one-off flags. Technically, I think the behaviour we have is that everything is "HostRelease" (even if the flag isn't set), and we need to turn it off with your added ClientRelease flag. Can we just make things behave that way? Alternatively, make the OGL TetureHost check for HostRelease, and clear the flag appropriately for canvas clients. As for the extra memory usage, yeah, but we'll have to live with it for now. Let's make it work first and then make it better :)

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #9) > Cool, that's nice and simple. But I see we already have: > > 35 // The host is responsible for tidying up any shared resources. > 36 const TextureFlags HostRelease = 0x20; > > which is only used here: > > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ > TextureClient.cpp#56 > I think the usage of "HostRelease" and "ClientRelease" are different. The ContentClientDoubleBuffered doesn't require the HostRelease flag. http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ContentClient.cpp#149 http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ContentClient.cpp#277 But it still needs the texture host to call DestroySharedSurface to free gralloc buffer. Otherwise, it causes the memory leak. http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ContentHost.cpp#321 Actually we can translate the "borrowing" concept to the ownership of gralloc buffer concept. And it is better to change "ClientRelease" to "OwnbyClient" to extend the usage of gralloc buffer. > We should rationalize these, otherwise we're introducing just one-off flags. > Technically, I think the behaviour we have is that everything is > "HostRelease" (even if the flag isn't set), and we need to turn it off with > your added ClientRelease flag. Can we just make things behave that way? > Alternatively, make the OGL TetureHost check for HostRelease, and clear the > flag appropriately for canvas clients. > > As for the extra memory usage, yeah, but we'll have to live with it for now. > Let's make it work first and then make it better :)

attached ownbyclient version

I incorporated that patch into the one in bug 843599.

fixed in bug 843599, case closed.