[llvm] [orc-rt] Make the Session attach preconditions a contract (PR #217325)

[Lang Hames via llvm-commits]

https://github.com/lhames created https://github.com/llvm/llvm-project/pull/217325

Session::attach silently dropped the attach if the Session was already attached, or if a detach or shutdown had been requested, leaving the ControllerAccess constructed but never connected or notified and giving the caller no indication. There is no coherent alternative behaviour -- a detach may be arbitrarily far along by then -- so document the precondition and assert it instead.

Also make Session::detach pass nullptr to proceedToDetach from its Start-state branch, asserting that no ControllerAccess is attached, to match Session::shutdown on the same path.

A TODO covers what to do about contract violations in release builds.

>From 4af9678f1de6f88c6ce27d69fb1e89495ea5a961 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Wed, 19 Aug 2026 18:16:03 +1000
Subject: [PATCH] [orc-rt] Make the Session attach preconditions a contract

Session::attach silently dropped the attach if the Session was already
attached, or if a detach or shutdown had been requested, leaving the
ControllerAccess constructed but never connected or notified and giving
the caller no indication. There is no coherent alternative behaviour --
a detach may be arbitrarily far along by then -- so document the
precondition and assert it instead.

Also make Session::detach pass nullptr to proceedToDetach from its
Start-state branch, asserting that no ControllerAccess is attached, to
match Session::shutdown on the same path.

A TODO covers what to do about contract violations in release builds.
---
 orc-rt/include/orc-rt/Session.h | 10 ++++++++++
 orc-rt/lib/executor/Session.cpp | 23 +++++++++++++++++------
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/orc-rt/include/orc-rt/Session.h b/orc-rt/include/orc-rt/Session.h
index 9a8fe68eecfc1..43ffcf8775a34 100644
--- a/orc-rt/include/orc-rt/Session.h
+++ b/orc-rt/include/orc-rt/Session.h
@@ -434,6 +434,13 @@ class Session {
   /// ControllerAccessT is constructed with a reference to this Session as its
   /// first argument, followed by the given args, as required by the
   /// ControllerAccess base constructor.
+  ///
+  /// A Session may be attached at most once, and attach must not be called
+  /// after -- or concurrently with -- detach or shutdown: by the time a detach
+  /// has been requested it may be arbitrarily far along, so there is no point
+  /// at which a newly attached controller could be connected, or its
+  /// disconnection coherently reported. Violating this is a programming error,
+  /// checked by assertion.
   template <typename ControllerAccessT, typename... ArgTs>
   void attach(BootstrapInfo BI, ArgTs &&...Args) {
     doAttach(std::make_shared<ControllerAccessT>(*this,
@@ -455,6 +462,9 @@ class Session {
   ///
   /// ControllerAccessT::Create is passed a reference to this Session as its
   /// first argument, followed by the given args.
+  ///
+  /// The attach itself is subject to the same restrictions as
+  /// attach<ControllerAccessT>; the returned Error reports Create failure only.
   template <typename ControllerAccessT, typename... ArgTs>
   Error tryAttach(BootstrapInfo BI, ArgTs &&...Args) {
     auto CA = ControllerAccessT::Create(*this, std::forward<ArgTs>(Args)...);
diff --git a/orc-rt/lib/executor/Session.cpp b/orc-rt/lib/executor/Session.cpp
index 1a435ad291767..820b5117c17c4 100644
--- a/orc-rt/lib/executor/Session.cpp
+++ b/orc-rt/lib/executor/Session.cpp
@@ -76,10 +76,17 @@ void Session::doAttach(std::shared_ptr<ControllerAccess> CA, BootstrapInfo BI) {
 
   {
     std::scoped_lock<std::mutex> Lock(M);
-    // Controller can only be attached from the start state if no
-    // other operation has been requested.
-    if (CurrentState != State::Start || TargetState != State::None)
-      return;
+    // A controller can only be attached from the start state, with no other
+    // operation requested: a Session is attached at most once, and attach must
+    // not be called after -- or concurrently with -- detach or shutdown. See
+    // the Session::attach contract.
+    //
+    // TODO: Settle on a policy for contract violations in release builds
+    // (probably abort) and apply it here. Without the assertions below a
+    // violating attach proceeds, clobbering TargetState and potentially
+    // regressing CurrentState from Detached back to Attached.
+    assert(CurrentState == State::Start && TargetState == State::None &&
+           "attach raced detach / shutdown, or Session already attached");
     assert(std::atomic_load(&this->CA) == nullptr &&
            "ControllerAccess object already attached?");
     std::atomic_store(&this->CA, CA);
@@ -160,9 +167,13 @@ void Session::detach(OnDetachFn OnDetach) {
       TmpCA = std::atomic_load(&this->CA);
     } else {
       assert(CurrentState == State::Start);
+      // A CA is only ever stored with TargetState raised to Attached, and
+      // TargetState is not lowered back to None until CurrentState reaches
+      // Attached, so reaching the Start state here implies no CA was attached.
+      assert(std::atomic_load(&this->CA) == nullptr &&
+             "Start state, but a ControllerAccess is attached?");
       // No controller was ever attached, so the disconnect trivially succeeds.
-      proceedToDetach(Lock, std::atomic_exchange(&this->CA, {}),
-                      Error::success());
+      proceedToDetach(Lock, nullptr, Error::success());
       return;
     }
   }