Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cpp/src/barrier/barrier.cu
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,12 @@ class iteration_data_t {
}
}

~iteration_data_t()
{
chol.reset();
handle_ptr->sync_stream();
}
Comment on lines +443 to +447

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Destructor can throw, risking abrupt termination.

The destructor calls handle_ptr->sync_stream(), which likely wraps cudaStreamSynchronize() and can throw raft::cuda_error on failure. If an exception escapes from a destructor during stack unwinding (e.g., when another exception is already active), C++ calls std::terminate(), causing an abrupt program crash instead of graceful shutdown.

Best practice: destructors should not throw. Wrap potentially-throwing calls in try/catch, log the error, and suppress the exception to allow cleanup to continue.

🐛 Proposed fix: Add exception handling in destructor
 ~iteration_data_t()
 {
-  chol.reset();
-  handle_ptr->sync_stream();
+  try {
+    chol.reset();
+    handle_ptr->sync_stream();
+  } catch (const std::exception& e) {
+    // Log but suppress: destructors must not throw
+    // settings_ is not accessible here, so we cannot log via settings_.log
+    // In production, consider using a global error handler or stderr
+    fprintf(stderr, "Warning: iteration_data_t destructor failed during cleanup: %s\n", e.what());
+  } catch (...) {
+    fprintf(stderr, "Warning: iteration_data_t destructor failed during cleanup (unknown exception)\n");
+  }
 }

Note: If settings_ or a logger is accessible in the destructor context, prefer logging through the proper channel instead of fprintf(stderr, ...).

As per coding guidelines: "cuOpt uses exceptions for error handling" and "Flag missing RAII in exception paths."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/barrier/barrier.cu` around lines 443 - 447, The destructor
~iteration_data_t() currently calls chol.reset() and handle_ptr->sync_stream()
which may throw (e.g., raft::cuda_error); wrap the call to
handle_ptr->sync_stream() in a try/catch block that catches all exceptions,
suppresses them, and logs the error (using the available logger if accessible or
fallback to fprintf(stderr, ...)) so no exception can escape the destructor;
ensure chol.reset() remains, perform sync_stream() inside the try, and do not
rethrow.


void form_augmented(bool first_call = false)
{
i_t n = A.n;
Expand Down
7 changes: 7 additions & 0 deletions python/cuopt/cuopt/linear_programming/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
# SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

import ctypes
import os

_gomp_path = os.path.join(os.path.dirname(__file__), "_libs", "libgomp-855c301a.so.1.0.0")
if os.path.exists(_gomp_path):
ctypes.CDLL(_gomp_path, mode=ctypes.RTLD_GLOBAL)
Comment on lines +7 to +9

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error handling for libgomp preload failures.

If the bundled libgomp exists but fails to load (due to corruption, incompatible architecture, or permission issues), ctypes.CDLL() will raise an exception and prevent the entire cuopt.linear_programming module from importing. Consider wrapping the load in a try/except block with a user-actionable error message explaining why the module failed to initialize.

🛡️ Proposed fix: Add try/except with actionable error message
 _gomp_path = os.path.join(os.path.dirname(__file__), "_libs", "libgomp-855c301a.so.1.0.0")
 if os.path.exists(_gomp_path):
-    ctypes.CDLL(_gomp_path, mode=ctypes.RTLD_GLOBAL)
+    try:
+        ctypes.CDLL(_gomp_path, mode=ctypes.RTLD_GLOBAL)
+    except OSError as e:
+        raise ImportError(
+            f"Failed to preload bundled OpenMP runtime library. "
+            f"This may indicate a corrupted installation. "
+            f"Please reinstall cuopt. Details: {e}"
+        ) from e
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/cuopt/cuopt/linear_programming/__init__.py` around lines 7 - 9, Wrap
the preloading of the bundled libgomp in a try/except so import failures don’t
crash the cuopt.linear_programming module: check for existence of _gomp_path as
before, then call ctypes.CDLL(_gomp_path, mode=ctypes.RTLD_GLOBAL) inside a try
block, catch Exception, and log or raise a clear, user-actionable error that
includes the _gomp_path and the exception message (or fallback to continuing
without preload), referencing the existing symbol names _gomp_path and
ctypes.CDLL so the change is easy to locate.


from cuopt.linear_programming import internals
from cuopt.linear_programming.data_model import DataModel
from cuopt.linear_programming.io import ParseMps, Read
Expand Down