Skip to content

🐛 Fix intermittent deadlock in remeshing_im - #94

Draft
jdumas wants to merge 1 commit into
mainfrom
jdumas/repro-remeshing-im-deadlock
Draft

🐛 Fix intermittent deadlock in remeshing_im#94
jdumas wants to merge 1 commit into
mainfrom
jdumas/repro-remeshing-im-deadlock

Conversation

@jdumas

@jdumas jdumas commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Context

The windows-2025 (Debug) job of #93 failed with:

637/637 Test #582: remeshing_im::remesh ***Timeout 1500.03 sec

The ctest log shows this is a hang, not a slow test:

02:17:47  Start 582: remeshing_im::remesh
02:17:47  582: [warning] Input mesh is too coarse ... subdividing ..   <- section 1
02:18:02  582: [warning] Input mesh is too coarse ... subdividing ..   <- section 2
02:42:47  ***Timeout 1500.03 sec

Section 1 finished in ~15s, so a proportionally-slow run would total ~50s. Instead there is zero
output for the full remaining 1500s. Note this job runs no sanitizer, so sanitizer overhead is
not involved.

Root cause

Optimizer::optimizeOrientations() / optimizePositions() set the mOptimizeOrientations /
mOptimizePositions predicate flags without holding mRes.mutex(), while the optimizer worker
reads those same flags under that mutex in Optimizer::run():

while (mRunning && (mRes.levels() == 0 || (!mOptimizePositions && !mOptimizeOrientations)))
    mCond.wait(mRes.mutex());

This opens a lost-wakeup window:

  1. The worker evaluates its wait predicate while holding mRes.mutex() and decides to sleep.
  2. It calls mCond.wait(), which only acquires the condition variable's internal mutex a few
    instructions later.
  3. If the main thread sets a flag and calls notify_all() inside that gap, the notification is
    delivered before the worker registers as a waiter, and is lost.
  4. The worker then sleeps forever, and the main thread's Optimizer::wait() sleeps waiting for a
    flag the worker will never clear. Deadlock.

The main thread can only slip into that gap because it writes the flag without the mutex. Note
that condition_variable_any is otherwise robust here: its internal mutex serialises notify_all()
against waiter registration. The window is only a handful of instructions wide, which is why this
reproduces so rarely.

Fix

instant-meshes-core is an external pinned dependency, so the fix is caller-side in remesh.cpp:
set the optimizer flags while holding mRes.mutex(). That closes the window, because the worker
holds the same mutex continuously from its predicate check until cv_any::wait() releases it.

ordered_lock is a non-recursive ticket lock and the flag setters take no lock themselves, so
holding the mutex across these calls introduces no self-deadlock.

Validation

The full 20-job matrix passed on this branch, including the
ubuntu-24.04 (gcc, Debug, ThreadSanitizer) job that previously timed out in xatlas (that one was
an unrelated lost-wakeup hang in the xatlas task scheduler, fixed by the dependency bump in #93):

585/637 Test #582: remeshing_im::remesh ......  Passed  158.42 sec
619/637 Test #622: rejects non-triangle mesh .  Passed    0.12 sec

Brute-force repetition did not reproduce the hang, as expected given how narrow the window is:

  • 50 serial iterations of the test on windows-2025 (Debug) in CI: all passed, very consistently at
    ~15.3s each.
  • 3000 local iterations under 12-way CPU contention: all passed.

ThreadSanitizer does pin it down deterministically, on the exact pair of accesses in the analysis
above. Against an unfixed build:

WARNING: ThreadSanitizer: data race
  #0 instant_meshes::Optimizer::run()
  #0 instant_meshes::Optimizer::optimizeOrientations(int)
SUMMARY: ThreadSanitizer: data race in instant_meshes::Optimizer::run()

4 such warnings before the change, 0 after it.

Note that .github/tsan.suppressions.ini currently hides this behind a broad race:instant_meshes::
entry, which already notes that Instant Meshes "has other data races in its Optimizer class" — so
the TSan CI job would not have caught this. Narrowing that suppression would be a reasonable
follow-up, but it likely surfaces other pre-existing races in the same library.

Known remaining issue

Optimizer::shutdown() has the same unsynchronised pattern:

mRunning = false;
notify();
mThread.join();

That one cannot be fixed from the call site, since the join() is internal to the optimizer. It
needs an upstream patch to instant-meshes-core.

@jdumas
jdumas force-pushed the jdumas/repro-remeshing-im-deadlock branch from 7f2d797 to 76ca89b Compare September 8, 2026 14:36
@jdumas
jdumas changed the base branch from main to jdumas/v6.48.0 September 8, 2026 14:36
Base automatically changed from jdumas/v6.48.0 to main September 8, 2026 17:53
Optimizer::optimizeOrientations() and optimizePositions() set the mOptimizeOrientations /
mOptimizePositions predicate flags without holding mRes.mutex(), while the optimizer worker
reads those same flags under that mutex in Optimizer::run().

This opens a lost-wakeup window. The worker evaluates its wait predicate while holding
mRes.mutex(), then calls mCond.wait(), which only acquires the condition variable's internal
mutex a few instructions later. If the main thread sets a flag and calls notify_all() inside
that gap, the notification is delivered before the worker registers as a waiter and is lost.
The worker then sleeps forever, and the main thread's Optimizer::wait() sleeps waiting for a
flag the worker will never clear.

Setting the flags while holding mRes.mutex() closes the window, since the worker holds that
same mutex continuously from its predicate check until cv_any::wait() releases it.

ThreadSanitizer reports the race between Optimizer::run() and Optimizer::optimizeOrientations()
on an unfixed build and reports none after this change.

ordered_lock is a non-recursive ticket lock and the flag setters take no lock themselves, so
holding the mutex across these calls introduces no self-deadlock.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jdumas
jdumas force-pushed the jdumas/repro-remeshing-im-deadlock branch 2 times, most recently from 96a5f85 to 58e6aaf Compare September 8, 2026 20:41
@qnzhou

qnzhou commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

I have incorporated this fix upstream (+ a few other tsan/asan fixes): qnzhou/instant-meshes-core#5

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.

2 participants