Skip to content

Commit 4afecf0

Browse files
committed
Fix counter logic, mem leaks and bugs in CProxyDirect3DDevice9.cpp
Fixes for: - Memory leaks - QueryInterface reference leaks - Double release in Release() method - Missing AddRef in constructor - Reference counter logic
1 parent 68ae075 commit 4afecf0

File tree

1 file changed

+89
-19
lines changed

1 file changed

+89
-19
lines changed

Client/core/DXHook/CProxyDirect3DDevice9.cpp

Lines changed: 89 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,17 @@ CProxyDirect3DDevice9::CProxyDirect3DDevice9(IDirect3DDevice9* pDevice)
2121
{
2222
WriteDebugEvent(SString("CProxyDirect3DDevice9::CProxyDirect3DDevice9 %08x (device: %08x)", this, pDevice));
2323

24-
// Set our wrapped device.
24+
// Set wrapped device - DO NOT AddRef here since we'll do it in AddRef method
2525
m_pDevice = pDevice;
26+
27+
// Initialize reference count to 1 (the creator holds a reference)
28+
m_ulRefCount = 1;
29+
30+
// Now add a reference to the underlying device to match initial ref count
31+
if (m_pDevice)
32+
{
33+
m_pDevice->AddRef();
34+
}
2635

2736
// Get CDirect3DData pointer.
2837
m_pData = CDirect3DData::GetSingletonPtr();
@@ -39,12 +48,18 @@ CProxyDirect3DDevice9::CProxyDirect3DDevice9(IDirect3DDevice9* pDevice)
3948
int iAdapter = creationParameters.AdapterOrdinal;
4049

4150
IDirect3D9* pD3D9 = CProxyDirect3D9::StaticGetDirect3D();
51+
bool bNeedRelease = false;
4252
if (!pD3D9)
53+
{
4354
m_pDevice->GetDirect3D(&pD3D9);
55+
bNeedRelease = true; // GetDirect3D increments reference count
56+
}
4457

45-
D3DADAPTER_IDENTIFIER9 adaptIdent;
46-
ZeroMemory(&adaptIdent, sizeof(D3DADAPTER_IDENTIFIER9));
47-
pD3D9->GetAdapterIdentifier(iAdapter, 0, &adaptIdent);
58+
if (pD3D9)
59+
{
60+
D3DADAPTER_IDENTIFIER9 adaptIdent;
61+
ZeroMemory(&adaptIdent, sizeof(D3DADAPTER_IDENTIFIER9));
62+
pD3D9->GetAdapterIdentifier(iAdapter, 0, &adaptIdent);
4863

4964
int iVideoCardMemoryKBTotal = GetWMIVideoAdapterMemorySize(adaptIdent.VendorId, adaptIdent.DeviceId) / 1024;
5065

@@ -59,6 +74,25 @@ CProxyDirect3DDevice9::CProxyDirect3DDevice9(IDirect3DDevice9* pDevice)
5974
//
6075
g_pDeviceState->AdapterState.Name = adaptIdent.Description;
6176

77+
// Clipping is required for some graphic configurations
78+
g_pDeviceState->AdapterState.bRequiresClipping = SStringX(adaptIdent.Description).Contains("Intel");
79+
80+
// Release D3D9 interface if we obtained it via GetDirect3D
81+
if (bNeedRelease)
82+
{
83+
SAFE_RELEASE(pD3D9);
84+
}
85+
}
86+
else
87+
{
88+
WriteDebugEvent("Warning: Could not obtain IDirect3D9 interface for adapter info");
89+
90+
// Set default values
91+
g_pDeviceState->AdapterState.InstalledMemoryKB = m_pDevice->GetAvailableTextureMem() / 1024;
92+
g_pDeviceState->AdapterState.Name = "Unknown";
93+
g_pDeviceState->AdapterState.bRequiresClipping = false;
94+
}
95+
6296
//
6397
// Get max anisotropic setting
6498
//
@@ -74,9 +108,6 @@ CProxyDirect3DDevice9::CProxyDirect3DDevice9(IDirect3DDevice9* pDevice)
74108
g_pDeviceState->AdapterState.MaxAnisotropicSetting++;
75109
}
76110

77-
// Clipping is required for some graphic configurations
78-
g_pDeviceState->AdapterState.bRequiresClipping = SStringX(adaptIdent.Description).Contains("Intel");
79-
80111
WriteDebugEvent(SString("*** Using adapter: %s (Mem:%d KB, MaxAnisotropy:%d)", (const char*)g_pDeviceState->AdapterState.Name,
81112
g_pDeviceState->AdapterState.InstalledMemoryKB, g_pDeviceState->AdapterState.MaxAnisotropicSetting));
82113

@@ -91,36 +122,66 @@ CProxyDirect3DDevice9::CProxyDirect3DDevice9(IDirect3DDevice9* pDevice)
91122
CProxyDirect3DDevice9::~CProxyDirect3DDevice9()
92123
{
93124
WriteDebugEvent(SString("CProxyDirect3DDevice9::~CProxyDirect3DDevice9 %08x", this));
94-
g_pDeviceState = NULL;
95-
g_pProxyDevice = NULL;
125+
126+
// Release our reference to the wrapped device
127+
// Note: We don't check m_ulRefCount here because destructor is only called
128+
// when Release() determined the count reached 0
129+
if (m_pDevice)
130+
{
131+
m_pDevice->Release();
132+
m_pDevice = nullptr;
133+
}
134+
135+
if (g_pProxyDevice == this)
136+
{
137+
g_pDeviceState = NULL;
138+
g_pProxyDevice = NULL;
139+
}
96140
}
97141

98142
/*** IUnknown methods ***/
99143
HRESULT CProxyDirect3DDevice9::QueryInterface(REFIID riid, void** ppvObj)
100144
{
145+
if (!ppvObj)
146+
return E_POINTER;
147+
148+
*ppvObj = nullptr;
149+
150+
// Check if its for IUnknown or IDirect3DDevice9
151+
if (riid == IID_IUnknown || riid == IID_IDirect3DDevice9)
152+
{
153+
*ppvObj = static_cast<IDirect3DDevice9*>(this);
154+
AddRef();
155+
return S_OK;
156+
}
157+
158+
// For any other interface, forward to underlying device
159+
// But this means the caller gets the underlying device, not our proxy
101160
return m_pDevice->QueryInterface(riid, ppvObj);
102161
}
103162

104163
ULONG CProxyDirect3DDevice9::AddRef()
105164
{
106-
return m_pDevice->AddRef();
165+
// Only increment proxy reference count
166+
// We keep a single reference to the underlying device throughout the lifetime
167+
return InterlockedIncrement(&m_ulRefCount);
107168
}
108169

109170
ULONG CProxyDirect3DDevice9::Release()
110171
{
111-
// Check if will be final release
112-
m_pDevice->AddRef();
113-
ULONG ulRefCount = m_pDevice->Release();
114-
if (ulRefCount == 1)
172+
// Only decrement proxy reference count
173+
ULONG ulNewRefCount = InterlockedDecrement(&m_ulRefCount);
174+
175+
if (ulNewRefCount == 0)
115176
{
116177
WriteDebugEvent("Releasing IDirect3DDevice9 Proxy...");
117178
// Call event handler
118179
CDirect3DEvents9::OnDirect3DDeviceDestroy(m_pDevice);
119180
delete this;
120-
return ulRefCount - 1;
181+
return 0;
121182
}
122183

123-
return m_pDevice->Release();
184+
return ulNewRefCount;
124185
}
125186

126187
/*** IDirect3DDevice9 methods ***/
@@ -314,15 +375,24 @@ HRESULT CProxyDirect3DDevice9::Reset(D3DPRESENT_PARAMETERS* pPresentationParamet
314375
// Store actual present parameters used
315376
IDirect3DSwapChain9* pSwapChain = nullptr;
316377
HRESULT hrSwapChain = m_pDevice->GetSwapChain(0, &pSwapChain);
317-
if (SUCCEEDED(hrSwapChain) && pSwapChain)
378+
if (SUCCEEDED(hrSwapChain))
318379
{
319-
pSwapChain->GetPresentParameters(&g_pDeviceState->CreationState.PresentationParameters);
320-
SAFE_RELEASE(pSwapChain);
380+
if (pSwapChain)
381+
{
382+
pSwapChain->GetPresentParameters(&g_pDeviceState->CreationState.PresentationParameters);
383+
}
384+
else
385+
{
386+
WriteDebugEvent("Warning: GetSwapChain succeeded but returned null swap chain");
387+
}
321388
}
322389
else
323390
{
324391
WriteDebugEvent(SString("Warning: Failed to get swap chain for parameter storage: %08x", hrSwapChain));
325392
}
393+
394+
// Always release the swap chain if it was obtained, regardless of success/failure
395+
SAFE_RELEASE(pSwapChain);
326396

327397
// Store device creation parameters as well
328398
HRESULT hrCreationParams = m_pDevice->GetCreationParameters(&g_pDeviceState->CreationState.CreationParameters);

0 commit comments

Comments
 (0)