Skip to content

cuOptSetLogCallback registration is process-global; concurrent solves can drop or leak a callback #1752

Closed

Description

@ramakrishnap-nv

Follow-up to review feedback on #1636 (raised by CodeRabbit, endorsed by @mlubin: "this seems like a real issue").

Problem

cuOptSetLogCallback stores a callback per settings object, but it reaches the
logger through a single process-global slot in cpp/src/utilities/logger.cpp:

static log_callback_with_data_t g_pending_callback = nullptr;
static void* g_pending_callback_data               = nullptr;

Flow: cuOptSolve drops the settings' callback into these globals via
set_pending_log_callback(), and init_logger_t picks it up, copies it into the
logger guard, and installs a callback_sink_mt.

Two failure modes follow, both only with concurrent or overlapping solves.

1. Clobbering. The mutex makes each write atomic, but not the
drop-then-consume pair. Thread A calls set_pending_log_callback(cbA), thread B
calls set_pending_log_callback(cbB) before A's init_logger_t runs, and A
silently gets B's callback.

2. Silent drop, with a stale pointer left behind. init_logger_t returns
early when a guard is already alive:

auto existing_guard = g_active_guard.lock();
if (existing_guard) { guard_ = existing_guard; return; }

So if solve A is running, solve B's init_logger_t never reads the mailbox: B's
callback is never installed, and B's registration stays in the globals. A later
solve can then pick it up and invoke B's function pointer with B's user_data,
which may already have been freed by then.

The second is the more serious one — it is not "some log lines go missing" but a
stale callback plus stale user data surviving the solve that registered them.

Root cause

default_logger() is a process-wide singleton with one global sink list, so
there is no per-solve context for user_log_bridge to route on. Per-solve
callback delivery needs solve identity threaded through the logging path.

Options

  1. Fail loudly. If a solve begins while a guard is active with a different
    callback, return an error from cuOptSolve rather than silently dropping it.
    Small and honest; rejects a legitimate use case.
  2. Thread-local registration. The bridge looks up a thread_local callback,
    so each solving thread gets its own. Covers lines emitted on the solve thread;
    lines from internal worker threads would reach no callback.
  3. Per-solve logger context. Thread solve identity through the logging path
    so the bridge can route correctly. Correct, and the largest change.

Whichever is chosen, clearing the pending slot on the early-return path would
remove the stale-pointer hazard on its own, and is worth doing regardless.

Scope

Single-solve use — the common case — is correct today, which is why #1636 ships
with this documented as a known limitation rather than blocked on it.

Activity

  1. chris-maes commented on Aug 20, 2026

    @chris-maes
    Contributor

    @ramakrishnap-nv concurrent solves are quite common. I don't think we should merge cuOptSetLogCallback with this issue.

  2. ramakrishnap-nv commented on Aug 20, 2026

    @ramakrishnap-nv
    CollaboratorAuthor

    Hmm, I assumed like multiple solver calls, but yeah, I didn't account for multiple internal solvers. Let me try to address this in the same PR.

  3. added a commit that references this issue on Aug 20, 2026
    6562dda
  4. added this to the 26.10 milestone on Aug 25, 2026
  5. added a commit that references this issue on Sep 2, 2026
    3636c72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions