patterncppMinor

D3D9 leaks if any?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

anyleaksd3d9

Problem

I am writing a D3D9 hook plugin to give back to a community that helped me learn programming. It is a community based on programming and scripting for games.

The plugin is supposed to hook d3d9.dll and allow clients to read pixels from the backbuffer. That is the entire extent of the plugin.

Thus I have hooked only the EndScene function.

I have also looked into the following functions to find their release methods:

IDirect3DDevice9::CreateOffscreenPlainSurface -> IDirect3DSurface9::Release

IDirect3DDevice9::GetRenderTarget -> IDirect3DSurface9::Release

IDirect3DDevice9::GetRenderTargetData -> No Release Method

IDirect3DSurface9::GetDesc -> No destroy method

IDirect3DSurface9::GetDC -> IDirect3DSurface9::ReleaseDC

So that ends my research of finding where a "possible" leak might be.

Now I have the following code that creates a texture for drawing, draws the texture and immediately frees it. I also have the following for reading the back-buffer into an array provided to my plugin via JNI. This array is guaranteed to be safe and released since it is created in a Java client.

All of my Direct-X code is as follows:

Definitions & Structures:

#define VERTEX_FVF (D3DFVF_XYZRHW | D3DFVF_DIFFUSE)
#define VERTEX_FVF_TEX (D3DFVF_XYZRHW | D3DFVF_DIFFUSE | D3DFVF_TEX1)

struct D3DVertex
{
    float X, Y, Z, RHW;
    unsigned int Colour;
    float U, V;
};

/**
    Calls the Release function of specified classes:
        Textures, Devices, etc..
**/
template
void SafeRelease(T* &ptr)
{
    if (ptr)
    {
        ptr->Release();
        ptr = nullptr;
    }
}

LoadTexture:

```
/**
Creates a texture from a given buffer.
Texture must be freed using SafeRelease.
**/
void LoadTexture(IDirect3DDevice9 Device, std::uint8_t buffer, int width, int height, IDirect3DTexture9* &Texture)
{
Device->CreateTexture(width, height, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_MANAGED, &Texture, 0);

D3DLOCKED_RECT rect;
Texture->LockRect(0, &

Solution

Coding style:

This is a list of improvements you can apply to make your code nicer
in a C++ context:

-
Prefer enums or numerical constants over macros. VERTEX_FVF and VERTEX_FVF_TEX
should either be members of an enum or const unsigned int objects.

-
You did well by using std::uint8_t but you forgot to do the same for
memcpy in the LoadTexture() function. memcpy for C++ is defined
in the header `. So it should be referred to as std::memcpy

.

- 
Avoid C-style casts. In most places you've correctly used the C++
cast operators. There are still a few C casts that should be removed
for full consistency. In

DrawTexture()

 there is such an instance.

- 
Use more

const

. Const can also be applied to local variables that
are meant to be assigned once. This is a conceptual kind of const,
unlike the one in an explicit numerical constant, which is a compile time
constant. In

DrawTexture() for example, UOffset, VOffset and Vertices


are init-once, never change variables. It is a good practice to mark those
as

const

.

- 
Read-only pointers are

const. In LoadTexture(), buffer

 is a pointer
to read-only memory in that context. It should then be a

const std::uint8_t* buffer

.

- 
Use more assertions. Most of your functions take pointers that are not
being validated before use. A good rule of thumb when dealing with pointer
parameters is to at least validate them with an

assert(p != nullptr)


before using them. You should apply that to your function parameters of pointer type.

- 
In the

DrawCircle() function, your value of π

 is used twice, so it
should be made a constant. You can define a local

static const float PI

 in function
scope for that. Alternatively, you could use the non-standard

M_PI

 constant
that is likely to be available on most compilers under the

 header.

- 
Most functions presented here are doing no error checking. Almost
all D3D calls will return you some error code. You are ignoring them most
of the time. A function like

LoadTexture()

, for instance, can easily fail
if the system is low on memory. If you don't care about returning an error
code to a higher level of the application, for whatever reasons, you should
at least check the error code inside the functions and log a message if
something failed. That will give you a hint of where to start looking for
the problem at a latter time. You can use

std::cerr

 for that.

- 
The last advice in this list that must be present for a C++ review is
about the procedural style of your program. Ideally, you should try to
model some aspects of your C++ program using Object Oriented Programming (OOP).
You seem to have a few globals in there, plus all the functions are declared
in the global scope. The global vars, in special, can be a source of bugs
and headache. If you could put some effort into making some parts of this code
more object-oriented, you could quite possibly get rid of the globals
and see some increase in the overall code quality and readability.

About the memory leaks:

At a glance, there doesn't seem to be any memory leaks in your code. D3D objects
don't throw exceptions, so you should be fine for that. If exceptions
are introduced at a later time though, that might also introduce memory leaks.

So how would you go about that? With smart pointers, of course!
Unfortunately, the smart pointers provided by the standard library are not customizable for used with COM objects.

I've never had such a need, but I imagine someone else must have had, since Microsoft makes heavy use of COM, so it is very likely that smart pointer libraries that support COM
objects do exist. You should try searching to find out.

In its simples form, however, a basic COM smart pointer could be implemented
as the following:

``
template
class COMRefPtr {

public:

RefPtr(T * object = nullptr)
: ptr(object) { }

RefPtr(const RefPtr & rhs)
: ptr(rhs.ptr)
{
if (ptr != nullptr)
{
ptr->AddRef();
}
}

RefPtr & operator = (T * object)
{
if (ptr != object)
{
T * tmp = ptr;
ptr = object;
if (tmp)
{
tmp->Release();
}
}
return (*this);
}

RefPtr & operator = (const RefPtr & rhs)
{
if (ptr != rhs.ptr)
{
T * tmp = ptr;
ptr = rhs.ptr;
if (ptr)
{
ptr->AddRef();
}
if (tmp)
{
tmp->Release();
}
}
return (*this);
}

T & operator *()
{
assert(ptr != nullptr && "Null pointer dereference!");
return *ptr;
}

const T & operator *() const
{
assert(ptr != nullptr && "Null pointer dereference!");
return *ptr;
}

T * operator->()
{
assert(ptr != nullptr && "Null pointer access!");
return ptr;
}

const T * operat

Code Snippets

template<class T>
class COMRefPtr {

public:

    RefPtr(T * object = nullptr)
        : ptr(object) { }

    RefPtr(const RefPtr & rhs)
        : ptr(rhs.ptr)
    {
        if (ptr != nullptr)
        {
            ptr->AddRef();
        }
    }

    RefPtr & operator = (T * object)
    {
        if (ptr != object)
        {
            T * tmp = ptr;
            ptr = object;
            if (tmp)
            {
                tmp->Release();
            }
        }
        return (*this);
    }

    RefPtr & operator = (const RefPtr & rhs)
    {
        if (ptr != rhs.ptr)
        {
            T * tmp = ptr;
            ptr = rhs.ptr;
            if (ptr)
            {
                ptr->AddRef();
            }
            if (tmp)
            {
                tmp->Release();
            }
        }
        return (*this);
    }

    T & operator *()
    {
        assert(ptr != nullptr && "Null pointer dereference!");
        return *ptr;
    }

    const T & operator *() const
    {
        assert(ptr != nullptr && "Null pointer dereference!");
        return *ptr;
    }

    T * operator->()
    {
        assert(ptr != nullptr && "Null pointer access!");
        return ptr;
    }

    const T * operator->() const
    {
        assert(ptr != nullptr && "Null pointer access!");
        return ptr;
    }

    bool operator!() const
    {
        // Allows a check such as "if (!ptr) ..."
        return ptr == nullptr;
    }

    // Access the raw pointer without changing the ref count.
    // Use these to interface with functions that take a raw pointer.
    T * get() { return ptr; }
    const T * get() const { return ptr; }

    // You will probably also want to provide comparison operators.
    // At least `==` and `!=`.

    ~RefPtr()
    {
        // Automatic cleanup.
        if (ptr != nullptr)
        {
            ptr->Release();
        }
    }

private:

    T * ptr;
};

Context

StackExchange Code Review Q#45058, answer score: 6

Revisions (0)

No revisions yet.