-
Notifications
You must be signed in to change notification settings - Fork 831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[d3d9] Fix for crashes on BACK buffer allocation failure due to OOM #2964
[d3d9] Fix for crashes on BACK buffer allocation failure due to OOM #2964
Conversation
…f Memory (doitsujin#9) Details: To avoid nullptr access on back buffer, its allocation failure exception is now catched to return D3DERR_OUTOFVIDEOMEMORY error on IDirect3DDevice9::Reset. Also on Back buffers destroy and Present paths, back buffer pointer validation was added to avoid accessing nullptr and crash.
void D3D9SwapChainEx::ValidateBackBuffersPtrs() { | ||
for (uint32_t i = 0; i < m_backBuffers.size(); i++) { | ||
if (!GetBackBuffer(i)) | ||
{ | ||
throw DxvkError("D3D9SwapChainEx: Back buffer pointer is invalid - nullptr"); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be possible to hit unless the app ignores the Reset error and goes on trying to present black frame buffers...
If you want to sanity check, just check m_backBuffers[0]
being non-null at present time and log+error, all of them don't need to be checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be possible to hit unless the app ignores the Reset error and goes on trying to present black frame buffers...
Yes, agree, that makes sense, but there is a test, which behaves this way and goes with Present despite of OOM error on Reset.
If you want to sanity check, just check
m_backBuffers[0]
being non-null at present time and log+error, all of them don't need to be checked.
Actually, in my scenario, only m_backBuffers[1] is null, m_backBuffers[0] is fine, so unless we're okay with crash on Present called after failed Reset, we should validate both backbuffers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove the reference to your internal issue in the commit message? It points to a random old issue on this tracker 🙂
Details:
To avoid nullptr access on back buffer, its allocation failure exception is now catched to return D3DERR_OUTOFVIDEOMEMORY error on IDirect3DDevice9::Reset. Also on Back buffers destroy and Present paths, back buffer pointer validation was added to avoid accessing nullptr and crash.