Skip to content

Guard Havok world objects against release while their world holds them - #2

Closed
viktor-ferenczi wants to merge 10 commits into
mainfrom
havok-dispose-guard
Closed

viktor-ferenczi wants to merge 10 commits into
mainfrom
havok-dispose-guard

Conversation

@viktor-ferenczi

Copy link
Copy Markdown
Contributor

Ports the Havok world-object dispose guard out of the Remote plugin (SE1-0009), as a Shared patch so the client and the dedicated server both get it.

The bug

HkHandle releases its native object both from Dispose() and from its finalizer, and HkWorld keeps no managed reference to the phantoms added to it. A rigid body or phantom whose managed wrapper is dropped without a RemoveRigidBody/RemovePhantom is therefore released while the broadphase still holds it, and the next broadphase pass walks freed memory.

Nothing about that is platform specific — it is managed lifetime handling, present on Windows, and the DS steps the same worlds through the same wrapper — so it belongs here rather than in linux-compat or in a test-harness plugin.

The fix

  • An explicit Dispose() of a world object removes it from its world first, then lets the release proceed.
  • The finalizer of one skips the native release entirely. A leak is far cheaper than a freed body in the broadphase, and the finalizer thread must not touch the world.
  • AddPhantom/RemovePhantom keep a native pointer -> world map, because phantoms are not entities and have no world pointer to read back. A disposed HkWorld drops its entries, which would otherwise outlive it and match whatever allocation reuses the pointer.

Two deliberate changes over the version that lived in Remote: the explicit-dispose path can now actually skip the release it claims to skip (Remote's void prefix discarded that decision and freed the body anyway), and the world-dispose purge above is new.

The patches are applied one at a time after PatchAll rather than by it, each in its own try/catch and every target named with an explicit argument list. HkHandle has both Dispose() and Dispose(bool), and an ambiguous target there aborts the whole PatchAll — in Remote that once took every patch in the plugin down at once. A bugfix must not be able to disable its siblings.

Testing

Verified on both arms, armed and silent:

  • Dedicated server (Magnetar, Earth world, no clients): ~9 min including a shutdown-with-save. All four targets resolve against the DS assemblies.
  • Client (headless Pulsar): world load, three ships closed, ~7000 frames at sim speed 1.0, then a full session reload.

The repair logic itself is untested. Every run above took the early-return path, so none of the four remedial branches has ever executed. The guard has not been observed to fire since the MXCSR FTZ/DAZ fix in the PE loader resolved T-0068's root cause, and there is no instrumented fleet left to campaign — so this goes in as a fix that is correct by inspection, not one with a reproduction behind it. If a campaign on the current wrapper leaves the counts at zero, deleting it is still the right call.

When a client joins, the server answers its vicinity-cache request with the
model paths taken from the server's own MyModel.AssetName values. Vanilla
assets are Content-relative and resolve on any client, but mod assets are the
server's absolute on-disk paths, which name nothing on the machine receiving
them. Every one of them fails as a missing mesh asset, so the prewarm the
vicinity cache exists for never happens for modded blocks near the spawn point.

Both sides hold the same published workshop items, so only the layout above the
mod folder differs. A prefix on MySession.PreloadVicinityCache replaces that
prefix with this client's own folder for the mod, matching on the published
file id. What cannot be matched and is still rooted names a location on the
sender's disk, usable only when hosting, so it is kept when it exists locally
and dropped otherwise; the preload then becomes a no-op instead of a failed
load. Content-relative paths are untouched.

Confirmed to be a game bug rather than a loader or platform one: joining
SpiroGames-JunkYard Paradise from Windows produced identical failures with both
the vanilla game and Pulsar, for the same three mods.
Remap server mod model paths in the vicinity asset preload
ClientPlugin/Config.cs still used the deleted Settings subsystem, which
broke the client build. The example patches are template stubs; the server
one also skipped LCD surface visibility updates on the DS.
HkHandle releases its native object both from Dispose() and from its finalizer,
and HkWorld keeps no managed reference to the phantoms added to it, so a rigid
body or phantom whose managed wrapper is dropped without a RemoveRigidBody /
RemovePhantom is released while the broadphase still holds it, and the next
broadphase pass walks freed memory. Nothing about that is platform specific: it
is managed lifetime handling, present on Windows, and the dedicated server steps
the same worlds through the same wrapper, so this goes in Shared.

An explicit dispose of a world object now removes it from its world first, and
the finalizer of one skips the native release entirely - a leak is far cheaper
than a freed body in the broadphase, and the finalizer thread must not touch the
world. Phantoms are not entities and have no world pointer to read back, so
AddPhantom/RemovePhantom keep a native pointer -> world map; a disposed HkWorld
drops its entries, which would otherwise outlive it and match whatever
allocation reuses the pointer.

Ported from the Remote plugin (SE1-0009), without its T-0068 experiment framing:
no stack-trace dumps and no finalizer census, just the first few cases of each
kind and a 60 s rollup while the counts keep moving. The patches are applied
one by one after PatchAll rather than by it, each in its own try/catch and every
target named with an explicit argument list, because HkHandle has both Dispose()
and Dispose(bool) and an ambiguous target there aborts the whole PatchAll - in
Remote that once took every patch in the plugin down at once.

Verified on both arms with the guard armed and silent: a Magnetar dedicated
server on the Earth world (~9 min, load, simulate, shutdown with save) and a
headless Pulsar client (world load, three ships closed, ~7000 frames, session
reload). It has not been observed to fire on a wrapper carrying the MXCSR
FTZ/DAZ fix that resolved T-0068's root cause.
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.

1 participant