[llvm] [orc-rt] Add Session::tryAttach, construct ControllerAccess in attach. (PR #207114)
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 1 21:07:28 PDT 2026
https://github.com/lhames created https://github.com/llvm/llvm-project/pull/207114
Reworks Session's controller-attachment API so that clients no longer construct or hold a ControllerAccess directly:
- attach<ControllerAccessT>(BI, Args...) now constructs the ControllerAccess internally, passing *this as the first constructor argument. Suitable for ControllerAccess implementations whose construction cannot fail.
- tryAttach<ControllerAccessT>(BI, Args...) is the new fallible counterpart to attach. It forwards *this and the given args to ControllerAccessT::Create, which must return an Expected<std::shared_ptr<ControllerAccessT>>. On success, proceeds to call connect on the instance, otherwise returns the Error. This lets implementations surface setup failures (e.g. failing to bind a socket) synchronously as an Error, without ever handing back a usable-but-unconnected object.
- The raw attach(shared_ptr<ControllerAccess>, BI) entry point becomes the private implementation detail doAttach.
Also adds ControllerAccessTest.TryAttach{Success,Failure} covering the new factory-based path, and updates existing call sites to the new API.
>From de8872be48f04d368be8408eefea413bf6491b35 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Thu, 2 Jul 2026 12:27:30 +1000
Subject: [PATCH] [orc-rt] Add Session::tryAttach, construct ControllerAccess
in attach.
Reworks Session's controller-attachment API so that clients no longer construct
or hold a ControllerAccess directly:
- attach<ControllerAccessT>(BI, Args...) now constructs the ControllerAccess
internally, passing *this as the first constructor argument. Suitable for
ControllerAccess implementations whose construction cannot fail.
- tryAttach<ControllerAccessT>(BI, Args...) is the new fallible counterpart to
attach. It forwards *this and the given args to ControllerAccessT::Create,
which must return an Expected<std::shared_ptr<ControllerAccessT>>. On
success, proceeds to call connect on the instance, otherwise returns the
Error. This lets implementations surface setup failures (e.g. failing to
bind a socket) synchronously as an Error, without ever handing back a
usable-but-unconnected object.
- The raw attach(shared_ptr<ControllerAccess>, BI) entry point becomes the
private implementation detail doAttach.
Also adds ControllerAccessTest.TryAttach{Success,Failure} covering the new
factory-based path, and updates existing call sites to the new API.
---
orc-rt/include/orc-rt/Session.h | 62 +++++++++---
orc-rt/lib/executor/Session.cpp | 4 +-
.../InProcessControllerAccessTest.cpp | 4 +-
orc-rt/unittests/SessionTest.cpp | 98 ++++++++++++++-----
4 files changed, 126 insertions(+), 42 deletions(-)
diff --git a/orc-rt/include/orc-rt/Session.h b/orc-rt/include/orc-rt/Session.h
index 54f7bf5f96bf7..7100dfe824672 100644
--- a/orc-rt/include/orc-rt/Session.h
+++ b/orc-rt/include/orc-rt/Session.h
@@ -277,26 +277,46 @@ class Session {
return addService(std::move(*Srv));
}
- /// Initiate connection with controller, using the given BootstrapInfo.
+ /// Construct a ControllerAccessT and immediately attach using the given
+ /// BootstrapInfo.
///
- /// Upon first call, assuming that the Session has not already been detached
- /// or shutdown, this will take (shared) ownership of CA and call its connect
- /// method.
+ /// This enables one-line attach operations in the common case where the
+ /// ControllerAccess implementation requires no further configuration after
+ /// construction and cannot fail to construct. ControllerAccess
+ /// implementations whose setup can fail (e.g. binding a socket) should
+ /// provide a Create factory and use tryAttach instead.
///
- /// If detach or shutdown have already been called then this method will not
- /// take ownership of CA or call its connect method.
- void attach(std::shared_ptr<ControllerAccess> CA, BootstrapInfo BI);
+ /// 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.
+ template <typename ControllerAccessT, typename... ArgTs>
+ void attach(BootstrapInfo BI, ArgTs &&...Args) {
+ doAttach(std::make_shared<ControllerAccessT>(*this,
+ std::forward<ArgTs>(Args)...),
+ std::move(BI));
+ }
- /// Construct a ControllerAccessT with the given args, then immediately
- /// attach using the given BootstrapInfo.
+ /// Try to construct a ControllerAccessT by forwarding a reference to this
+ /// Session and the given args to ControllerAccessT::Create, which must
+ /// return an Expected<std::shared_ptr<ControllerAccessT>>. On success,
+ /// immediately attaches using the given BootstrapInfo.
///
- /// This enables one-line attach operations in the common case where the
- /// ControllerAccess implementation does not require any further
- /// configuration after construction.
+ /// This is the fallible counterpart to attach<ControllerAccessT>: it allows
+ /// ControllerAccess implementations to surface setup failures (e.g. failure
+ /// to bind a socket) synchronously as an Error, without ever handing the
+ /// caller a usable-but-unconnected ControllerAccess object. Runtime and
+ /// remote failures should still be reported asynchronously via
+ /// notifyDisconnected.
+ ///
+ /// ControllerAccessT::Create is passed a reference to this Session as its
+ /// first argument, followed by the given args.
template <typename ControllerAccessT, typename... ArgTs>
- void attach(BootstrapInfo BI, ArgTs &&...Args) {
- attach(std::make_shared<ControllerAccessT>(std::forward<ArgTs>(Args)...),
- std::move(BI));
+ Error tryAttach(BootstrapInfo BI, ArgTs &&...Args) {
+ auto CA = ControllerAccessT::Create(*this, std::forward<ArgTs>(Args)...);
+ if (!CA)
+ return CA.takeError();
+ doAttach(std::move(*CA), std::move(BI));
+ return Error::success();
}
/// Initiate detach from the controller.
@@ -453,6 +473,18 @@ class Session {
void appendService(std::unique_ptr<Service> Srv);
+ /// Attach the given ControllerAccess, using the given BootstrapInfo.
+ ///
+ /// Upon first call, assuming that the Session has not already been detached
+ /// or shutdown, this takes (shared) ownership of CA and calls its connect
+ /// method. If detach or shutdown have already been called then this method
+ /// will not take ownership of CA or call its connect method.
+ ///
+ /// This is an implementation detail of the public attach / tryAttach
+ /// templates, which are responsible for constructing the ControllerAccess
+ /// object: clients never hold a ControllerAccess directly.
+ void doAttach(std::shared_ptr<ControllerAccess> CA, BootstrapInfo BI);
+
void handleDisconnect();
void proceedToDetach(std::unique_lock<std::mutex> &Lock,
std::shared_ptr<ControllerAccess> TmpCA);
diff --git a/orc-rt/lib/executor/Session.cpp b/orc-rt/lib/executor/Session.cpp
index d153c55cea1be..e9e7207819f24 100644
--- a/orc-rt/lib/executor/Session.cpp
+++ b/orc-rt/lib/executor/Session.cpp
@@ -63,8 +63,8 @@ Session::~Session() {
});
}
-void Session::attach(std::shared_ptr<ControllerAccess> CA, BootstrapInfo BI) {
- assert(CA && "attach called with null CA object");
+void Session::doAttach(std::shared_ptr<ControllerAccess> CA, BootstrapInfo BI) {
+ assert(CA && "doAttach called with null CA object");
{
std::scoped_lock<std::mutex> Lock(M);
diff --git a/orc-rt/unittests/InProcessControllerAccessTest.cpp b/orc-rt/unittests/InProcessControllerAccessTest.cpp
index 88c7ce0e28b72..9ea5dcb2b3110 100644
--- a/orc-rt/unittests/InProcessControllerAccessTest.cpp
+++ b/orc-rt/unittests/InProcessControllerAccessTest.cpp
@@ -110,7 +110,7 @@ class MockIPEPC {
// MockIPEPC into MockOut from inside OnConnect.
void attachWithMock(Session &S, std::unique_ptr<MockIPEPC> &MockOut) {
S.attach<InProcessControllerAccess>(
- BootstrapInfo(S), S,
+ BootstrapInfo(S),
[&MockOut](InProcessControllerAccess &, BootstrapInfo &,
InProcessControllerAccess::Connection *C,
InProcessControllerAccess::BootstrapInfoAccess *) -> Error {
@@ -161,7 +161,7 @@ TEST(InProcessControllerAccessTest, OnConnectFailureIsReportedAndDetaches) {
[&](Error E) { Reported = std::move(E); });
S.attach<InProcessControllerAccess>(
- BootstrapInfo(S), S,
+ BootstrapInfo(S),
[](InProcessControllerAccess &, BootstrapInfo &,
InProcessControllerAccess::Connection *,
InProcessControllerAccess::BootstrapInfoAccess *) -> Error {
diff --git a/orc-rt/unittests/SessionTest.cpp b/orc-rt/unittests/SessionTest.cpp
index 7455fc4c182ef..28e4fffca14e8 100644
--- a/orc-rt/unittests/SessionTest.cpp
+++ b/orc-rt/unittests/SessionTest.cpp
@@ -90,11 +90,28 @@ class MockControllerAccess : public Session::ControllerAccess {
/// sides.
using PostFn = move_only_function<void(move_only_function<void()>)>;
- MockControllerAccess(Session &SS, PostFn Post = {})
- : Session::ControllerAccess(SS), Post(std::move(Post)) {}
+ MockControllerAccess(Session &SS, PostFn Post = {},
+ OnConnectFn OnConnect = {},
+ MockControllerAccess **Self = nullptr)
+ : Session::ControllerAccess(SS), Post(std::move(Post)),
+ OnConnect(std::move(OnConnect)) {
+ // Optionally publish this instance so tests that need to drive the
+ // controller side directly can reach it after attach constructs it.
+ // (attach constructs the ControllerAccess internally and does not hand
+ // back a reference, since the object may not outlive the attach call.)
+ if (Self)
+ *Self = this;
+ }
- void setOnConnect(OnConnectFn OnConnect) {
- this->OnConnect = std::move(OnConnect);
+ /// Fallible named constructor for testing tryAttach. Returns an error if
+ /// Fail is true, otherwise a MockControllerAccess forwarding the remaining
+ /// arguments to the constructor.
+ static Expected<std::shared_ptr<MockControllerAccess>>
+ Create(Session &S, bool Fail, PostFn Post = {}, OnConnectFn OnConnect = {}) {
+ if (Fail)
+ return make_error<StringError>("failed to create controller access");
+ return std::make_shared<MockControllerAccess>(S, std::move(Post),
+ std::move(OnConnect));
}
void connect(BootstrapInfo BI) override {
@@ -645,8 +662,7 @@ TEST(ControllerAccessTest, Basics) {
// down as expected.
QueueingRunner<>::WorkQueue Tasks;
Session S(mockExecutorProcessInfo(), QueueingRunner(Tasks), noErrors);
- S.attach(std::make_shared<MockControllerAccess>(S, postOnto(Tasks)),
- BootstrapInfo(S));
+ S.attach<MockControllerAccess>(BootstrapInfo(S), postOnto(Tasks));
QueueingRunner<>::runFIFOUntilEmpty(Tasks);
}
@@ -665,8 +681,7 @@ TEST(ControllerAccessTest, ValidCallToController) {
// Simulate a call to a controller handler.
QueueingRunner<>::WorkQueue Tasks;
Session S(mockExecutorProcessInfo(), QueueingRunner(Tasks), noErrors);
- S.attach(std::make_shared<MockControllerAccess>(S, postOnto(Tasks)),
- BootstrapInfo(S));
+ S.attach<MockControllerAccess>(BootstrapInfo(S), postOnto(Tasks));
int32_t Result = 0;
SPSWrapperFunction<int32_t(int32_t, int32_t)>::call(
@@ -699,8 +714,7 @@ TEST(ControllerAccessTest, CallToControllerAfterDetach) {
// Expect calls to the controller prior to attaching to fail.
QueueingRunner<>::WorkQueue Tasks;
Session S(mockExecutorProcessInfo(), QueueingRunner(Tasks), noErrors);
- S.attach(std::make_shared<MockControllerAccess>(S, postOnto(Tasks)),
- BootstrapInfo(S));
+ S.attach<MockControllerAccess>(BootstrapInfo(S), postOnto(Tasks));
S.detach();
@@ -720,8 +734,9 @@ TEST(ControllerAccessTest, CallFromController) {
// Simulate a call from the controller.
QueueingRunner<>::WorkQueue Tasks;
Session S(mockExecutorProcessInfo(), QueueingRunner(Tasks), noErrors);
- auto CA = std::make_shared<MockControllerAccess>(S, postOnto(Tasks));
- S.attach(CA, BootstrapInfo(S));
+ MockControllerAccess *CA = nullptr;
+ S.attach<MockControllerAccess>(BootstrapInfo(S), postOnto(Tasks),
+ MockControllerAccess::OnConnectFn{}, &CA);
int32_t Result = 0;
SPSWrapperFunction<int32_t(int32_t, int32_t)>::call(
@@ -742,10 +757,9 @@ TEST(ControllerAccessTest, FailConnect) {
EXPECT_EQ(toString(std::move(Err)), ErrMsg);
});
BootstrapInfo BI(S);
- auto CA = std::make_shared<MockControllerAccess>(S);
- CA->setOnConnect(
+ S.attach<MockControllerAccess>(
+ std::move(BI), MockControllerAccess::PostFn{},
[&](BootstrapInfo &BI) { return make_error<StringError>(ErrMsg); });
- S.attach(std::move(CA), std::move(BI));
ASSERT_TRUE(GotError);
}
@@ -766,14 +780,52 @@ TEST(ControllerAccessTest, BootstrapInfoPassedToConnect) {
BI.values()[SecretKey] = SecretValue;
bool OnConnectRan = false;
- auto CA = std::make_shared<MockControllerAccess>(S);
- CA->setOnConnect([&](BootstrapInfo &BI) {
- EXPECT_EQ(BI.symbols().at(SymName), static_cast<const void *>(&Sym));
- EXPECT_EQ(BI.values().at(SecretKey), SecretValue);
- OnConnectRan = true;
- return Error::success();
- });
- S.attach(CA, std::move(BI));
+ S.attach<MockControllerAccess>(
+ std::move(BI), MockControllerAccess::PostFn{}, [&](BootstrapInfo &BI) {
+ EXPECT_EQ(BI.symbols().at(SymName), static_cast<const void *>(&Sym));
+ EXPECT_EQ(BI.values().at(SecretKey), SecretValue);
+ OnConnectRan = true;
+ return Error::success();
+ });
ASSERT_TRUE(OnConnectRan);
}
+
+TEST(ControllerAccessTest, TryAttachSuccess) {
+ // A successful Create attaches the controller, which then services calls
+ // just like one attached via attach<T>.
+ QueueingRunner<>::WorkQueue Tasks;
+ Session S(mockExecutorProcessInfo(), QueueingRunner(Tasks), noErrors);
+ cantFail(S.tryAttach<MockControllerAccess>(BootstrapInfo(S), /*Fail=*/false,
+ postOnto(Tasks)));
+
+ int32_t Result = 0;
+ SPSWrapperFunction<int32_t(int32_t, int32_t)>::call(
+ S.callViaSession(reinterpret_cast<Session::HandlerTag>(add_sps_wrapper)),
+ [&](Expected<int32_t> R) { Result = cantFail(std::move(R)); }, 41, 1);
+
+ QueueingRunner<>::runFIFOUntilEmpty(Tasks);
+
+ EXPECT_EQ(Result, 42);
+}
+
+TEST(ControllerAccessTest, TryAttachFailure) {
+ // A failing Create surfaces its Error and leaves the Session unattached.
+ Session S(mockExecutorProcessInfo(), noDispatch, noErrors);
+ auto Err = S.tryAttach<MockControllerAccess>(BootstrapInfo(S), /*Fail=*/true);
+ ASSERT_TRUE(static_cast<bool>(Err));
+ EXPECT_EQ(toString(std::move(Err)), "failed to create controller access");
+
+ // Since nothing was attached, calls to the controller should fail as they
+ // would before any attach.
+ Error CallErr = Error::success();
+ SPSWrapperFunction<int32_t(int32_t, int32_t)>::call(
+ S.callViaSession(reinterpret_cast<Session::HandlerTag>(add_sps_wrapper)),
+ [&](Expected<int32_t> R) {
+ ErrorAsOutParameter _(CallErr);
+ CallErr = R.takeError();
+ },
+ 41, 1);
+
+ EXPECT_EQ(toString(std::move(CallErr)), "no controller attached");
+}
More information about the llvm-commits
mailing list