[webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D

[Chris Marrin]

On Oct 11, 2010, at 4:34 PM, James Robinson wrote:

> 
> 
> On Mon, Oct 11, 2010 at 4:03 PM, Chris Marrin <cmarrin at apple.com> wrote:
> 
> On Oct 11, 2010, at 3:35 PM, James Robinson wrote:
> 
> > On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin <cmarrin at apple.com> wrote:
> >
> > ...
> > So is this something I should be concerned about, or is an implicit policy sufficient in this case?
> >
> > Before somebody suggests it, I think Chris and I are in agreement that neither GraphicsContext3D nor DrawingBuffer should be RefCounted.  They both have clear single-ownership semantics.
> 
> True, although Simon and I just chatted and he pointed out that Refcounting both classes would solve this problem. The fact that GraphicsContext3D wouldn't need a back pointer to DrawingBuffer means we wouldn't have any circular references. I don't think the overhead of refcounting is an issue here, so maybe that would be a simpler solution?
> 
> I'd really prefer not to make them RefCounted.  The problem is that GraphicsContext3D and DrawingBuffer are very heavyweight objects, which means we need to manage their lifetimes tightly and avoid leaving them lying around longer than necessary.  The exact resource use of these objects depends on the system but a GraphicsContext3D will typically be backed by an OpenGL context, which implies some set of driver resources, and a DrawingBuffer is normally backed by a few megabytes of VRAM.  In the current code, there is always a single object responsible for managing the lifetime of a given GraphicsContext3D or a DrawingBuffer which makes it very easy to ensure that they live for as long as necessary but no longer.  With a RefCounted object it can be difficult to ensure that all references to a given object go away when necessary.
> 
> In this particular case DrawingBuffer exists at a slightly higher abstraction layer than GraphicsContext3D.  DrawingBuffer depends on GC3D, but there is no dependency the other way (and IMHO there should not be).  This means that the lifetime of a DrawingBuffer depends on the underlying GraphicsContext3D, but not the other way 'round.  All callers that use or will want to use a DrawingBuffer already have to be aware of the lifetime of the GraphicsContext3D associated with it since the only way to use a DrawingBuffer is by using the GraphicsContext3D API.
> 
> For those following along at home, the current user of DrawingBuffer is CanvasRenderingContext2D which has an OwnPtr<DrawingBuffer> and uses a GraphicsContext3D that is guaranteed to outlive the CanvasRenderingContext2D.  The next proposed user of DrawingBuffer is WebGLRenderingContext which manages its own GraphicsContext3D via a member OwnPtr<>.

How can you make that guarantee? The GraphicsContext3D is owned by SharedGraphicsContext3D, which is owned by Page. When Page is destroyed, it will destroy the SharedGraphicsContext3D which will destroy the GraphicsContext3D. The associated DrawingBuffers are owned by CanvasRenderingContext2D which are owned by HTMLCanvasElement. These live in the DOM Tree, which should be destroyed when the Page is destroyed. But Elements are refcounted, so what if there is a JavaScript reference to the HTMLCanvasElement. Or what if it's referenced by some other mechanism which does deferred destruction (like a run loop observer or something). Do we guarantee that all these references will be removed before the Page is destroyed? If this is ever not the case and the DrawingBuffer ever happens to get destroyed after the GraphicsContext3D, you will get a crash. or worse.

When I say worse, I'm referring to the fact that some of the uses of these graphics resources rely on the graphics context being previously bound. So it's possible in these cases that the wrong context is bound. It's not inconceivable that an exploit can be found that tries to access a Canvas while destroying a Page. Time it just right and maybe you can get access to some pixels from another window, which is a pretty bad security hole.

It just feels safer to me to manage the lifetime of these objects explicitly rather than relying on a complex sequence of events.

-----
~Chris
cmarrin at apple.com