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.

Sign up for GitHub

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

Jump to bottom

[d3d9] Fix crash if device is freed with bound textures #3408

Merged
merged 1 commit into from
May 9, 2023
Merged

[d3d9] Fix crash if device is freed with bound textures #3408

merged 1 commit into from
May 9, 2023

Conversation

AlpyneDreams
Copy link
Contributor

The following code will cause a segfault (in pDevice->Release) in D3D9, but works fine on native:

IDirect3DDevice9* pDevice = nullptr;
HRESULT status = pD3D9->CreateDevice(...);

IDirect3DTexture9* pTexture = nullptr;
status = pDevice->CreateTexture(256, 256, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_MANAGED, &pTexture, nullptr);

status = pDevice->SetTexture(0, pTexture);

pTexture->Release();
pDevice->Release();

The reason is that in the device, m_state is freed after m_mappedTextures, but if any textures are bound and don't have any refs left, they'll be freed, and the destructor for D3D9CommonTexture will try to remove them from the list.

dxvk/src/d3d9/d3d9_common_texture.cpp

Lines 96 to 101 in bef2ef6

D3D9CommonTexture::~D3D9CommonTexture() {
if (m_size != 0)
m_device->ChangeReportedMemory(m_size);
m_device->RemoveMappedTexture(this);
}

Changing the order so that m_state is declared last fixes this issue.

@AlpyneDreams AlpyneDreams marked this pull request as ready for review May 8, 2023 05:08
@doitsujin
Copy link
Owner

doitsujin commented May 8, 2023 •
edited
Loading

Looks good, do we know if this bug affects any real-world games?

@misyltoad misyltoad merged commit 2b09932 into doitsujin:master May 9, 2023
@AlpyneDreams
Copy link
Contributor Author

@doitsujin

Looks good, do we know if this bug affects any real-world games?

It affected Warcraft III in d8vk. It would crash if you changed resolutions because that game recreates the device instead of resetting, and does it somewhat haphazardly.

@AlpyneDreams AlpyneDreams mentioned this pull request May 10, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@K0bin K0bin K0bin approved these changes

@doitsujin doitsujin doitsujin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlpyneDreams @doitsujin @K0bin @misyltoad