ddraw: [3/3] Fix for regression bug #13277

[Jens Albretsen]

On Saturday 24 May 2008 17:30:47 Stefan Dösinger wrote:
> > +    ref = getRefcount(lpDD);
> > +    ok(ref == 1, "Got refcount %ld, expected 2\n", ref);
> > +
> > +    IDirectDraw_Release(lpDD);
> > +
> > +    ref = getRefcount(lpDD);
> > +    ok(ref == 0, "Got refcount %ld, expected 1\n", ref);
>
> I think there's a copypaste error, it seems you forgot to adjust the error
> string.
Correct.

> Your test shows the correctness of the patch pretty well, although out of
> curiosity, can you try this: Between the IDirectDraw_Release(), which
> reduces the refcount to 0 and the ref = getRefcount(), can you try to
> HeapAlloc() small portions(e.g. 512 bytes) of memory with HEAP_ZERO_MEMORY
> until you get an out of memory error. This should make it less likely that
> the lpDD object survives by luck. It might be destroyed by windows, but the
> memory is not yet overwritten by something else, so the getRefcount works
> by luck. If this does not lead to a crash, try the memory allocation +
> getRefcount again after the Surface::Release call below, there it should
> cause a crash.
I understand.
You want this in the patch or just test it?
Is there a implementation of allocate all memory/free all memory?
I have a issue with the last part of the test, would it be ok for the test to 
crash on purpose, I mean the last test.

> > @@ -1998,6 +2026,13 @@ IDirectDrawImpl_CreateNewSurface(IDirectDrawImpl
>
> *This,
>
> >     InterlockedIncrement(&This->surfaces);
> >     list_add_head(&This->surface_list, &(*ppSurf)->surface_list_entry);
> >
> > +    IDirectDrawInternal_AddRef(This);
> >
> > @@ -424,6 +424,9 @@ IDirectDrawSurfaceImpl_Release(IDirectDrawSurface7
>
> *iface)
>
> >          /* Reduce the ddraw refcount */
> >          if(ifaceToRelease) IUnknown_Release(ifaceToRelease);
> >          LeaveCriticalSection(&ddraw_cs);
> > +
> > +        /* Surface does not need ddraw anymore, release it's reference
> > */ +        IDirectDrawInternal_Release(ddraw);
>
> The counterpart to IDirectDrawImpl_CreateNewSurface is
> IDirectDrawSurfaceImpl_Destroy, not _Release. CreateNewSurface is called
> for each complex sublevel surface(e.g. front+backbuffer if they are created
> via DDSC_COMPLEX), while IDirectDrawSurface::Release must be called only
> for the root surface(ie, the pointer returned from
> IDirectDraw::CreateSurface). That means you have to move the
> IDirectDrawInternal_Release into
> IDirectDrawSurfaceImpl_Destroy, otherwise there's a reference leak.

Consider it fixed.

/JensA