Skip to content

Fix Heap Alloction issues in QTFred#7334

Open
TheForce172 wants to merge 2 commits intoscp-fs2open:masterfrom
TheForce172:fix/QTFreHeapCrashes
Open

Fix Heap Alloction issues in QTFred#7334
TheForce172 wants to merge 2 commits intoscp-fs2open:masterfrom
TheForce172:fix/QTFreHeapCrashes

Conversation

@TheForce172
Copy link
Copy Markdown
Contributor

@TheForce172 TheForce172 commented Mar 30, 2026

There have been multiple heap crash issues with QTFred on that have been traced to the fact that QT Distributes its libraries with dynamic linking while we are using static linking. We most likely can't distribute statically linked version of QT due to licencing issues between QT and FSO's licenses. So are only option is to compile dynamically if we are building QTFRED.

Fixes Multiple QTFred heap crashes due to mismatched runtime types.
@wookieejedi wookieejedi added fix A fix for bugs, not-a-bugs, and/or regressions. qtfred A feature or issue related to qtFred. labels Mar 30, 2026
@github-project-automation github-project-automation bot moved this to Work In Progress (PRs) in qtFRED2 Mar 30, 2026
@wookieejedi wookieejedi added this to the Release 26.0 milestone Mar 30, 2026
Copy link
Copy Markdown
Contributor

@JohnAFernandez JohnAFernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anyone else is going to look at this. Hold on to your butts.

@github-project-automation github-project-automation bot moved this from Work In Progress (PRs) to In Review (PRs) in qtFRED2 Apr 3, 2026
@wookieejedi
Copy link
Copy Markdown
Member

Given Cyborg approved and TheE says it looks fine, I'll plan to merge tomorrow.

Copy link
Copy Markdown
Contributor

@Goober5000 Goober5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does look like the best solution, from a design standpoint, but this PR only touches MSVC. We'll need to build QtFRED dynamically regardless of whatever build environment we use or what OS we build on.

@notimaginative
Copy link
Copy Markdown
Contributor

This does look like the best solution, from a design standpoint, but this PR only touches MSVC. We'll need to build QtFRED dynamically regardless of whatever build environment we use or what OS we build on.

I'm pretty sure this is an issue specific to MSVC (or using that toolchain). So this shouldn't be a problem on other platforms and/or toolchains. I'm not sure whether it applies to cross-compilation though (don't think so?).

@Goober5000
Copy link
Copy Markdown
Contributor

This does look like the best solution, from a design standpoint, but this PR only touches MSVC. We'll need to build QtFRED dynamically regardless of whatever build environment we use or what OS we build on.

I'm pretty sure this is an issue specific to MSVC (or using that toolchain). So this shouldn't be a problem on other platforms and/or toolchains. I'm not sure whether it applies to cross-compilation though (don't think so?).

But licensing issues would apply regardless of platform. Or do the other platforms already link dynamically?

@notimaginative
Copy link
Copy Markdown
Contributor

notimaginative commented Apr 4, 2026

This does look like the best solution, from a design standpoint, but this PR only touches MSVC. We'll need to build QtFRED dynamically regardless of whatever build environment we use or what OS we build on.

I'm pretty sure this is an issue specific to MSVC (or using that toolchain). So this shouldn't be a problem on other platforms and/or toolchains. I'm not sure whether it applies to cross-compilation though (don't think so?).

But licensing issues would apply regardless of platform. Or do the other platforms already link dynamically?

Oh no, this isn't about linking with Qt, it's about how linking is done with the Windows runtime. Qt links the Windows runtime dynamically, while FSO links it statically. And you can't safely mix the two methods. And it's much easier to just change how FSO links than Qt does.

@Goober5000
Copy link
Copy Markdown
Contributor

Goober5000 commented Apr 4, 2026

Ok, fair enough.

EDIT: A quick search shows that MSVC_USE_RUNTIME_DLL is used in several places across the codebase, including to determine whether MFC itself is linked dynamically. I'm wondering if MSVC_USE_RUNTIME_DLL should be set based on FSO_BUILD_QTFRED, rather than adding this one special case here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix for bugs, not-a-bugs, and/or regressions. qtfred A feature or issue related to qtFred.

Projects

Status: In Review (PRs)

Development

Successfully merging this pull request may close these issues.

5 participants