Skip to content
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

Conversation

adamjer
Copy link
Contributor

@adamjer adamjer commented Sep 28, 2022

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.

…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.
Comment on lines +1173 to +1181
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");
}
}
}

Copy link
Collaborator

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.

Copy link

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.

Copy link
Contributor

@Blisto91 Blisto91 left a 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 🙂

@adamjer adamjer changed the title [DXVK] Fix for crashes on BACK buffer allocation failure due to Out o… [DXVK] Fix for crashes on BACK buffer allocation failure due to OOM Oct 10, 2022
@adamjer adamjer changed the title [DXVK] Fix for crashes on BACK buffer allocation failure due to OOM [d3d9] Fix for crashes on BACK buffer allocation failure due to OOM Mar 1, 2023
Joshua-Ashton added a commit that referenced this pull request May 3, 2023
Joshua-Ashton added a commit that referenced this pull request May 3, 2023
Joshua-Ashton added a commit that referenced this pull request May 3, 2023
Etaash-mathamsetty pushed a commit to Etaash-mathamsetty/dxvk that referenced this pull request Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants