Skip to content

Commit d25d926

Browse files
committed
[ESI][Runtime] Fix engine teardown oversight
Some engines need to touch accelerator resources (e.g. MMIO regions) in their `disconnect`. So we need to ensure that those resources persist though engine disconnect/destruction. Assisted-by: Githut-Copilot:Claude Opus 4.8
1 parent 73a1b32 commit d25d926

3 files changed

Lines changed: 21 additions & 4 deletions

File tree

lib/Dialect/ESI/runtime/cpp/include/esi/Accelerator.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ class AcceleratorConnection {
100100
Context &getCtxt() const { return ctxt; }
101101
Logger &getLogger() const { return ctxt.getLogger(); }
102102

103-
/// Disconnect from the accelerator cleanly.
103+
/// Disconnect from the accelerator cleanly. Drains owned engines before the
104+
/// accelerator is released. Backends _must_ call this from their destructor
105+
/// while still fully constructed (their vtable/resources alive), since engine
106+
/// teardown may reference accelerator-owned objects. Must be idempotent.
104107
virtual void disconnect();
105108

106109
/// Request a reset of the accelerator design. Returns true if the reset was

lib/Dialect/ESI/runtime/cpp/include/esi/Engines.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ class Engine {
4545
virtual ~Engine() = default;
4646
/// Start the engine, if applicable.
4747
virtual void connect() { connected = true; };
48-
/// Stop the engine, if applicable.
48+
/// Stop the engine, if applicable. Guaranteed to be called (via the
49+
/// connection's disconnect()) while the Accelerator and its MMIO regions are
50+
/// still alive, so port teardown here may safely touch them. This method
51+
/// _must_ be idempotent.
4952
virtual void disconnect() { connected = false; };
5053
/// Get a port for a channel, from the cache if it exists or create it. An
5154
/// engine may override this method if different behavior is desired.

lib/Dialect/ESI/runtime/cpp/lib/Accelerator.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,12 @@ AcceleratorConnection::takeOwnership(std::unique_ptr<Accelerator> acc) {
121121
}
122122

123123
void AcceleratorConnection::clearOwnedObjects() {
124-
ownedAccelerator.reset();
125-
serviceCache.clear();
124+
// Destroy engines (and the ports they own) before the accelerator they may
125+
// reference during teardown -- order matters.
126126
clientEngines.clear();
127127
ownedEngines.clear();
128+
serviceCache.clear();
129+
ownedAccelerator.reset();
128130
}
129131

130132
/// Get the path to the currently running executable.
@@ -499,10 +501,19 @@ void AcceleratorServiceThread::addPoll(HWModule &module) {
499501
}
500502

501503
void AcceleratorConnection::disconnect() {
504+
// Stop polling before tearing down engines.
502505
if (serviceThread) {
503506
serviceThread->stop();
504507
serviceThread.reset();
505508
}
509+
// Drain engines while the accelerator (and its MMIO regions/services) is
510+
// still alive, since engine/port teardown may touch accelerator-owned
511+
// resources. Idempotent: disconnect() may be called more than once (e.g.
512+
// explicitly and again from the destructor).
513+
for (auto &[idPath, engine] : ownedEngines)
514+
engine->disconnect();
515+
clientEngines.clear();
516+
ownedEngines.clear();
506517
}
507518

508519
} // namespace esi

0 commit comments

Comments
 (0)