Refcounting fun

[H. Verbeet]

I've run into a somewhat annoying problem with d3d9/wined3d recently.
It appears that on d3d9 on Windows a surface either forwards its
AddRef / Release calls to its container if it has one, or just shares
the refcount directly. Either way, it comes down to the same thing.
Calling  AddRef on the surface increases the texture's refcount and
calling AddRef on the texture increases the surface's refcount. Same
thing for Release.

As a consequence, on Windows, it's perfectly legal to do something like this:

/* Create a texture */
IDirect3DDevice9_CreateTexture()
/* Get a surface. Adds a reference to the surface/texture combination */
IDirect3DTexture9_GetSurfaceLevel()
/* Release the texture. Note that the texture isn't freed yet, because
we still hold a reference to the surface */
IDirect3DTexture9_Release()
<Do some stuff with the surface, perhaps call GetContainer, etc>
/* Release the surface. The texture and all of it's surfaces will now
be released. */
IDirect3DSurface9_Release()

In wine, the call to IDirect3DTexture9_Release() frees the texture.
(It also calls Release on it's surfaces, but since the surface had an
extra reference from the call to GetSurfaceLevel the surface is still
ok). Calling any function on the surface that needs access to the
container will now have undefines results, most likely a crash.

Naively, I tried to fix this by having the surface keep a reference to
it's container. That keeps the code above from crashing, but only
because it creates a circular reference between the surface and the
container. Oops.

Fixing the problem for wined3d is probably not even that hard. Just
separate the Release code from the Cleanup code, then have
WineD3DSurface forward its AddRef / Release calls to the container, if
it has one. The container can then call the Cleanup code for its
surfaces when it gets deleted.

However, there is another problem, related to the way d3d9 (and
ddraw/d3d7/d3d8, when their respective rewrites are finished) wraps
wined3d. After fixing the problem for wined3d as above, the relation
between IDirect3DSurface9 and IDirect3DTexture9 in wine would look
roughly like this:


    |-------------------|       |-------------------|
    | IDirect3DSurface9 |       | IDirect3DTexture9 |
    |-------------------|       |-------------------|
            ^                           ^
            |                           |
    |----------------|           |----------------|
    | WineD3DSurface |<--------->| WineD3DTexture |
    |----------------|           |----------------|

WineD3DSurface and WineD3DTexture "share" reference counts,
IDirect3DSurface9 keeps a reference to WineD3DSurface, and
IDirect3DTexture keeps a reference to WineD3DTexture. WineD3DSurface
and WineD3DTexture don't keep references to their parents (and can't,
without creating circular references).

So releasing the texture before the surface would still not work,
since the d3d9 texture still has only 1 reference, the one it got when
it was created. That means that in the call to
IDirect3DTexture9_Release the d3d9 texture gets freed, while the
wined3d texture doesn't. When we do something with the d3d9 surface
that needs the container, the wined3d surface correctly returns its
container, but when d3d9 then tries to get the parent of that
container it still dies.

One way to solve this would be to have wined3d handle the reference
counting for d3d7/8/9 as well, so that their AddRef/Release calls just
call AddRef/Release on the wined3d object they wrap. That would also
mean we need to pass a pointer to the d3d7/8/9 object's cleanup
function. (Releasing a d3d9 surface to 0 should cause a d3d9 texture
to be freed). Note that for creating a texture, currently a similair
construction is used, in that IWineD3DDeviceImpl_CreateTexture gets
passed a pointer to a d3d9 surface create function, in order to create
the texture's surfaces.

Obviously doing all that would be quite a change in the way d3d is
setup in wine, and would also have some implications for the d3d7 and
d3d8 rewrites. I'm wondering if this solution would be acceptable, and
whether perhaps there are other, better solutions. Anyone care to
comment?