Skip to content

NanoVDB: cudaCheck should throw instead of calling exit(), and teardown paths need a non-throwing check #2265

Description

@harrism

Problem

cudaCheck — NanoVDB's error-check macro, used in roughly 450 places — does not report errors to the caller. It calls exit().

// nanovdb/util/cuda/Util.h
static inline void gpuAssert(cudaError_t code, const char* file, int line, bool abort = true)
{
    if (code != cudaSuccess) {
        fprintf(stderr, "CUDA error %u: %s (%s:%d)\n", ...);
        if (abort) exit(code);
    }
}

#define cudaCheck(ans) { gpuAssert((ans), __FILE__, __LINE__); }

abort defaults to true and no call site passes false, so every checked CUDA failure terminates the host process. cudaCheckError() and cudaSync() route through the same path; checkPtr/ptrAssert call exit(1). gpuAssert is host-only — no __device__/__hostdev__ annotation, and calling cudaCheck from a kernel is a compile error — so this is purely a host-side API design question.

For anything embedding NanoVDB — a DCC plugin, a renderer, a Python extension, a long-running service — a recoverable CUDA error becomes an unrecoverable process kill with no chance to clean up, report, or fall back. It is also untestable: a unit test cannot assert that an error path is taken, because taking it kills the test runner.

This is inconsistent with how NanoVDB reports errors elsewhere: GridHandle throws std::runtime_error, std::logic_error and std::ios_base::failure, and cuda::Buffer (#2251) throws std::runtime_error on size overflow.

The concrete bug: exit() from destructors during shutdown

One macro is used for every context — allocation, teardown, debug-only sanity checks. Those contexts have different requirements, and collapsing them produces a real bug.

Destructors call cudaCheck on CUDA frees:

~UnifiedBuffer(){ cudaCheck(cudaFree(mPtr)); }                      // cuda/UnifiedBuffer.h
~DeviceBuffer() { this->clear(); }                                  // cuda/DeviceBuffer.h
//   clear() -> cudaCheck(cudaFreeHost(...)), cudaCheck(util::cuda::freeAsync(...))
~TempPool()     { mResource->deallocate_async(...); }               // cuda/TempPool.h
//   -> DeviceResource::deallocate_async -> cudaCheck(util::cuda::freeAsync(...))
~DeviceStreamMap(), ~DistributedPointsToGrid(), ~IndexToGrid(), ~unique_ptr()

If any of these is destroyed after the CUDA runtime has begun unloading — a static or global GridHandle, a Python extension module being torn down, any object outliving main() — the free returns cudaErrorCudartUnloading. That is a benign, expected shutdown condition. NanoVDB treats it as fatal and calls exit(4), turning a clean shutdown into a nonzero process exit long after the user's work completed successfully.

DeviceBuffer::operator=(DeviceBuffer&&) noexcept has the same exposure, and additionally means a naive "make cudaCheck throw" would call std::terminate there instead.

Prior art: how RMM handles this

RMM hit exactly this problem and solved it with a context-dependent split rather than one macro. From rmm/detail/error.hpp (main branch):

1. Throwing macros for paths that can propagate. RMM_CUDA_TRY throws rmm::cuda_error; RMM_CUDA_TRY_ALLOC throws rmm::out_of_memory for cudaErrorMemoryAllocation and rmm::bad_alloc otherwise — mapping CUDA failures onto the standard allocator contract so ordinary std::bad_alloc handlers work. Neither ever calls exit().

2. A non-throwing macro for paths that cannot throw. The doc comment states the rationale directly:

This utility should be used in situations where extra error checking is desired in "Debug" builds, or in situations where an error case cannot throw an exception (such as a class destructor).

#ifdef NDEBUG
#define RMM_ASSERT_CUDA_SUCCESS(_call) do { (_call); } while (0);
#else
#define RMM_ASSERT_CUDA_SUCCESS(_call)                                           \
  do {                                                                           \
    cudaError_t const status__ = (_call);                                        \
    if (status__ != cudaSuccess) { std::cerr << "CUDA Error detected. " << ... }  \
    assert(status__ == cudaSuccess);                                             \
  } while (0)
#endif

Debug builds log and assert; release builds still make the call but do not check it. Neither terminates the process.

3. A shutdown-specific variant treating the unloading error as success:

#define RMM_ASSERT_CUDA_SUCCESS_SAFE_SHUTDOWN(_call)                             \
    ...                                                                          \
    if (status__ != cudaSuccess && status__ != cudaErrorCudartUnloading) { ... }  \
    assert(status__ == cudaSuccess || status__ == cudaErrorCudartUnloading);

This macro treats cudaErrorCudartUnloading as success, which can occur when CUDA resources are released after the CUDA runtime has begun shutting down (e.g., during static object destruction after main() exits).

This is used in every resource's deallocatecuda_memory_resource, managed_memory_resource, pinned_host_memory_resource, cuda_async_view_memory_resource, cuda_stream, and the adaptors — while their allocate uses RMM_CUDA_TRY_ALLOC. The split is applied consistently by context.

RMM goes further with rmm::process_is_exiting() (rmm/process_is_exiting.hpp), a noexcept atomic load that never calls CUDA. Destructors consult it and skip CUDA calls entirely when the process is exiting, deliberately leaking and letting the OS reclaim — because calling CUDA APIs from destructors after main() returns is undefined behaviour, and the primary context may already be gone, so you crash inside libcuda rather than getting an error code back. (RMM scopes this to its own internal per-device resource maps rather than offering it as a general facility, so it is the least directly transferable of the three.)

Two smaller problems in the same block

The debug guard is dead:

// change 1 -> 0 to only perform asserts during debug builds
#if 1 || defined(DEBUG) || defined(_DEBUG)

The leading 1 || short-circuits, so checks are always compiled and the #else no-op branch is unreachable in every configuration.

The comment above the macro is wrong. It reads // ... No-op in release builds. — given the guard, it is never a no-op. This invites the assumption that checks cost nothing in release.

Why it surfaced now

cuda::Buffer (#2251) wraps its prefix copy in try/catch to free the new allocation if the copy fails. That handler is unreachable, because cudaCheck exits rather than throwing. The guard was kept — it is correct once cudaCheck throws, and it keeps the two resize overloads symmetric — but today it is dead code written against an error model the library does not have.

Proposed direction

cudaCheck should throw, and should never call exit(). A library has no business terminating its host application; that decision belongs to the application. This also makes error paths testable, brings cudaCheck in line with GridHandle and cuda::Buffer, which already throw, and makes the try/catch in #2251 meaningful.

Concretely:

  1. cudaCheck throws a cudaError_t-carrying exception (a nanovdb::CudaError deriving from std::runtime_error, keeping the existing file/line/message text). exit() is removed. Optionally follow RMM in mapping cudaErrorMemoryAllocation to something deriving from std::bad_alloc, so standard allocation-failure handlers work.

  2. Teardown paths use a separate non-throwing, shutdown-safe check that treats cudaErrorCudartUnloading as success — logging and asserting in debug, calling unchecked in release, exactly as RMM_ASSERT_CUDA_SUCCESS_SAFE_SHUTDOWN does. This is required for correctness regardless of item 1: throwing from a destructor or a noexcept move calls std::terminate, which is no better than exit().

    The migration is small. 16 of the ~450 call sites are inside destructors or noexcept move operations, across 6 functions (~UnifiedBuffer, ~DeviceStreamMap, ~DistributedPointsToGrid, ~IndexToGrid, ~unique_ptr, DeviceBuffer::operator=(DeviceBuffer&&)), plus a handful reached indirectly via DeviceBuffer::clear() and DeviceResource::deallocate_async(). Under 5% of sites need to change spelling; the remaining ~430 keep cudaCheck and simply stop calling exit(), which is the point of the change.

  3. Repair the #if 1 || guard and the stale comment, independent of 1 and 2.

Item 1 is a behavioural change for anyone currently relying on the abort, so it warrants a pendingchanges note and, arguably, a minor rather than patch version bump. Item 2 fixes a live bug and could land first on its own.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions