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

[Chris Marrin]

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:
> 
> For accelerated 2D rendering we created a class called DrawingBuffer. This encapsulates the accelerated drawing surface (usually in the GPU) and the compositing layer used to display that surface on the page. The drawing surface (which is called a Framebuffer Object or FBO) is allocated by GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently this is a weak reference. DrawingBuffer has a ::create() method and you pass the GraphicsContext3D to that method.
> 
> If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer. If you were to try to destroy the DrawingBuffer it would attempt to use that pointer (to destroy its FBO) and crash or worse. Currently we have an implicit policy that you should never destroy a GraphicsContext3D before its DrawingBuffers are all destroyed. That works fine in the current use case, CanvasRenderingContext2D. And we can follow that same policy when in the future when we use DrawingBuffer in WebGLRenderingContext.
> 
> My concern is that this sort of implicit policy can lead to errors in the future when other potential clients of these classes use them. So I posted https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to all the DrawingBuffers allocated so they can be cleaned up when GraphicsContext3D is destroyed. In talking to James R. he's concerned this adds unnecessary complexity and would rather stick with the implicit policy.
> 
> 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?

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