Skip to content

Asan build jolt memory bug catches [SYNTH-270]#1434

Open
BrandonPacewic wants to merge 3 commits into
devfrom
branp/270/jolt-memory-fixes
Open

Asan build jolt memory bug catches [SYNTH-270]#1434
BrandonPacewic wants to merge 3 commits into
devfrom
branp/270/jolt-memory-fixes

Conversation

@BrandonPacewic

@BrandonPacewic BrandonPacewic commented Jul 22, 2026

Copy link
Copy Markdown
Member

Task

SYNTH-270

Resolve as many "simple" jolt memory leaks and issues as could be found through empirical testing.

Symptom

Several instances of us not fully understanding some cases of jolt memory were causing us to miss things. Proper teardown of some objects never happened, some cases of true use after free bugs were present.

Solution

  • Fixed 4 static-alias free bugs.
  • Fixed 1 true use after free issue.
  • Fixed two different double free cases.
  • Fixed a uncounted amount of leaks throughout these files.

This list of memory bugs is exhaustive for the files included. With these changes the Asan build now passes which previously failed.

Verification

Every single change here was verified with a full Jolt Asan build and run. No remaining leaks are in the files present in this pr.

Verify that everything still works, this should be a no-op with current behavior. Test current known memory related bugs and crashes to see if this indirectly fixed any.


Before merging, ensure the following criteria are met:

  • All acceptance criteria outlined in the ticket are met.
  • Necessary test cases have been added and updated.
  • A feature toggle or safe disable path has been added (if applicable).
  • User-facing polish:
    • Ask: "Is this ready-looking?"
  • Cross-linking between Jira and GitHub:
    • PR links to the relevant Jira issue.
    • Jira ticket has a comment referencing this PR.

@BrandonPacewic BrandonPacewic self-assigned this Jul 22, 2026
@BrandonPacewic
BrandonPacewic requested review from a team as code owners July 22, 2026 20:41
@BrandonPacewic BrandonPacewic added the bug Fixes an issue label Jul 22, 2026

@Dhruv-0-Arora Dhruv-0-Arora left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2026-07-24 at 2 01 48 PM

Spawning robots doesn't quite work

Comment on lines -609 to +637
// TODO
// Figure out if this cast is necessary
// If not, replace with `this.newConstraint()`
const fixedConstraint = JOLT.castObject(fixedSettings.Create(bodyMain, bodyWheel), JOLT.TwoBodyConstraint)
const fixedConstraint = fixedSettings.Create(bodyMain, bodyWheel)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How come we are removing JOLT.castObject() ?
It looks to me as though it was a load-bearing cast

in SynthesisBrain, this is now dead:

        // Two body constraints are part of wheels and are used to determine which way a wheel is facing
        const fixedConstraints: Jolt.TwoBodyConstraint[] = this._mechanism.constraints
            .filter(mechConstraint => mechConstraint.primaryConstraint instanceof JOLT.TwoBodyConstraint)
            .map(mechConstraint => mechConstraint.primaryConstraint as Jolt.TwoBodyConstraint)

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

Labels

bug Fixes an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants