Merge changes Id0f3fe5a,I1657b15c,I80e69a93
* changes:
Create an ExitHandle interface
autocxx: Support multiple AUTOCXX_RS_JSON_ARCHIVE entries
autocxx: Generate impls for abstract types too
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 316ac29..2bcde2f 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -863,6 +863,28 @@
absl::btree_set<const Channel *> taken_watchers_, taken_senders_;
};
+// Interface for terminating execution of an EventLoop.
+//
+// Prefer this over binding a lambda to an Exit() method when passing ownership
+// in complicated ways because implementations should have assertions to catch
+// it outliving the object it's referring to, instead of having a
+// use-after-free.
+//
+// This is not exposed by EventLoop directly because different EventLoop
+// implementations provide this functionality at different scopes, or possibly
+// not at all.
+class ExitHandle {
+ public:
+ ExitHandle() = default;
+ virtual ~ExitHandle() = default;
+
+ // Exits some set of event loops. Details depend on the implementation.
+ //
+ // This means no more events will be processed, but any currently being
+ // processed will finish.
+ virtual void Exit() = 0;
+};
+
} // namespace aos
#include "aos/events/event_loop_tmpl.h" // IWYU pragma: export
diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index a43de78..467353e 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -509,6 +509,22 @@
SimpleShmFetcher simple_shm_fetcher_;
};
+class ShmExitHandle : public ExitHandle {
+ public:
+ ShmExitHandle(ShmEventLoop *event_loop) : event_loop_(event_loop) {
+ ++event_loop_->exit_handle_count_;
+ }
+ ~ShmExitHandle() override {
+ CHECK_GT(event_loop_->exit_handle_count_, 0);
+ --event_loop_->exit_handle_count_;
+ }
+
+ void Exit() override { event_loop_->Exit(); }
+
+ private:
+ ShmEventLoop *const event_loop_;
+};
+
class ShmSender : public RawSender {
public:
explicit ShmSender(std::string_view shm_base, EventLoop *event_loop,
@@ -1171,6 +1187,10 @@
void ShmEventLoop::Exit() { epoll_.Quit(); }
+std::unique_ptr<ExitHandle> ShmEventLoop::MakeExitHandle() {
+ return std::make_unique<ShmExitHandle>(this);
+}
+
ShmEventLoop::~ShmEventLoop() {
CheckCurrentThread();
// Force everything with a registered fd with epoll to be destroyed now.
@@ -1179,6 +1199,8 @@
watchers_.clear();
CHECK(!is_running()) << ": ShmEventLoop destroyed while running";
+ CHECK_EQ(0, exit_handle_count_)
+ << ": All ExitHandles must be destroyed before the ShmEventLoop";
}
void ShmEventLoop::SetRuntimeRealtimePriority(int priority) {
diff --git a/aos/events/shm_event_loop.h b/aos/events/shm_event_loop.h
index e51f21b..425d334 100644
--- a/aos/events/shm_event_loop.h
+++ b/aos/events/shm_event_loop.h
@@ -22,6 +22,7 @@
class ShmSender;
class SimpleShmFetcher;
class ShmFetcher;
+class ShmExitHandle;
} // namespace shm_event_loop_internal
@@ -48,6 +49,8 @@
// Exits the event loop. Async safe.
void Exit();
+ std::unique_ptr<ExitHandle> MakeExitHandle();
+
aos::monotonic_clock::time_point monotonic_now() const override {
return aos::monotonic_clock::now();
}
@@ -138,6 +141,7 @@
friend class shm_event_loop_internal::ShmSender;
friend class shm_event_loop_internal::SimpleShmFetcher;
friend class shm_event_loop_internal::ShmFetcher;
+ friend class shm_event_loop_internal::ShmExitHandle;
using EventLoop::SendTimingReport;
@@ -165,6 +169,8 @@
const UUID boot_uuid_;
+ int exit_handle_count_ = 0;
+
// Capture the --shm_base flag at construction time. This makes it much
// easier to make different shared memory regions for doing things like
// multi-node tests.
diff --git a/aos/events/shm_event_loop_test.cc b/aos/events/shm_event_loop_test.cc
index f4107b0..f119479 100644
--- a/aos/events/shm_event_loop_test.cc
+++ b/aos/events/shm_event_loop_test.cc
@@ -425,6 +425,14 @@
TestNextMessageNotAvailableNoRun(true);
}
+// Test that an ExitHandle outliving its EventLoop is caught.
+TEST_P(ShmEventLoopDeathTest, ExitHandleOutlivesEventLoop) {
+ auto loop1 = factory()->MakePrimary("loop1");
+ auto exit_handle = static_cast<ShmEventLoop *>(loop1.get())->MakeExitHandle();
+ EXPECT_DEATH(loop1.reset(),
+ "All ExitHandles must be destroyed before the ShmEventLoop");
+}
+
// TODO(austin): Test that missing a deadline with a timer recovers as expected.
INSTANTIATE_TEST_SUITE_P(ShmEventLoopCopyTest, ShmEventLoopTest,
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index 0bdd711..f570296 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -156,6 +156,23 @@
SimulatedChannel *simulated_channel_ = nullptr;
};
+class SimulatedFactoryExitHandle : public ExitHandle {
+ public:
+ SimulatedFactoryExitHandle(SimulatedEventLoopFactory *factory)
+ : factory_(factory) {
+ ++factory_->exit_handle_count_;
+ }
+ ~SimulatedFactoryExitHandle() override {
+ CHECK_GT(factory_->exit_handle_count_, 0);
+ --factory_->exit_handle_count_;
+ }
+
+ void Exit() override { factory_->Exit(); }
+
+ private:
+ SimulatedEventLoopFactory *const factory_;
+};
+
class SimulatedChannel {
public:
explicit SimulatedChannel(const Channel *channel,
@@ -1298,7 +1315,10 @@
}
}
-SimulatedEventLoopFactory::~SimulatedEventLoopFactory() {}
+SimulatedEventLoopFactory::~SimulatedEventLoopFactory() {
+ CHECK_EQ(0, exit_handle_count_)
+ << ": All ExitHandles must be destroyed before the factory";
+}
NodeEventLoopFactory *SimulatedEventLoopFactory::GetNodeEventLoopFactory(
std::string_view node) {
@@ -1480,6 +1500,10 @@
void SimulatedEventLoopFactory::Exit() { scheduler_scheduler_.Exit(); }
+std::unique_ptr<ExitHandle> SimulatedEventLoopFactory::MakeExitHandle() {
+ return std::make_unique<SimulatedFactoryExitHandle>(this);
+}
+
void SimulatedEventLoopFactory::DisableForwarding(const Channel *channel) {
CHECK(bridge_) << ": Can't disable forwarding without a message bridge.";
bridge_->DisableForwarding(channel);
diff --git a/aos/events/simulated_event_loop.h b/aos/events/simulated_event_loop.h
index 9d5d11f..22eedc1 100644
--- a/aos/events/simulated_event_loop.h
+++ b/aos/events/simulated_event_loop.h
@@ -27,6 +27,7 @@
class NodeEventLoopFactory;
class SimulatedEventLoop;
+class SimulatedFactoryExitHandle;
namespace message_bridge {
class SimulatedMessageBridge;
}
@@ -94,6 +95,8 @@
// loop handler.
void Exit();
+ std::unique_ptr<ExitHandle> MakeExitHandle();
+
const std::vector<const Node *> &nodes() const { return nodes_; }
// Sets the simulated send delay for all messages sent within a single node.
@@ -147,6 +150,7 @@
private:
friend class NodeEventLoopFactory;
+ friend class SimulatedFactoryExitHandle;
const Configuration *const configuration_;
EventSchedulerScheduler scheduler_scheduler_;
@@ -159,6 +163,8 @@
std::vector<std::unique_ptr<NodeEventLoopFactory>> node_factories_;
std::vector<const Node *> nodes_;
+
+ int exit_handle_count_ = 0;
};
// This class holds all the state required to be a single node.
diff --git a/aos/events/simulated_event_loop_test.cc b/aos/events/simulated_event_loop_test.cc
index 2d59c5d..2922460 100644
--- a/aos/events/simulated_event_loop_test.cc
+++ b/aos/events/simulated_event_loop_test.cc
@@ -2052,6 +2052,19 @@
EXPECT_DEATH({ factory.RunFor(dt * 2); }, "Event loop");
}
+// Test that an ExitHandle outliving its factory is caught.
+TEST(SimulatedEventLoopDeathTest, ExitHandleOutlivesFactory) {
+ aos::FlatbufferDetachedBuffer<aos::Configuration> config =
+ aos::configuration::ReadConfig(
+ ArtifactPath("aos/events/multinode_pingpong_test_split_config.json"));
+ auto factory = std::make_unique<SimulatedEventLoopFactory>(&config.message());
+ NodeEventLoopFactory *pi1 = factory->GetNodeEventLoopFactory("pi1");
+ std::unique_ptr<EventLoop> loop = pi1->MakeEventLoop("foo");
+ auto exit_handle = factory->MakeExitHandle();
+ EXPECT_DEATH(factory.reset(),
+ "All ExitHandles must be destroyed before the factory");
+}
+
// Tests that messages don't survive a reboot of a node.
TEST(SimulatedEventLoopTest, ChannelClearedOnReboot) {
aos::FlatbufferDetachedBuffer<aos::Configuration> config =
diff --git a/third_party/autocxx/engine/src/conversion/codegen_rs/mod.rs b/third_party/autocxx/engine/src/conversion/codegen_rs/mod.rs
index 5dd4cbb..9bea2e0 100644
--- a/third_party/autocxx/engine/src/conversion/codegen_rs/mod.rs
+++ b/third_party/autocxx/engine/src/conversion/codegen_rs/mod.rs
@@ -1042,6 +1042,7 @@
extern_c_mod_items: vec![
self.generate_cxxbridge_type(name, false, doc_attrs)
],
+ bridge_items: create_impl_items(&id, movable, destroyable, self.config),
bindgen_mod_items,
materializations,
..Default::default()
diff --git a/third_party/autocxx/gen/cmd/src/main.rs b/third_party/autocxx/gen/cmd/src/main.rs
index 8b109c3..f8db241 100644
--- a/third_party/autocxx/gen/cmd/src/main.rs
+++ b/third_party/autocxx/gen/cmd/src/main.rs
@@ -74,7 +74,11 @@
instead of
--gen-rs-include
and you will need to give AUTOCXX_RS_JSON_ARCHIVE when building the Rust code.
-The output filename is named gen.rs.json.
+The output filename is named gen.rs.json. AUTOCXX_RS_JSON_ARCHIVE should be set
+to the path to gen.rs.json. It may optionally have multiple paths separated the
+way as the PATH environment variable for the current platform, see
+[`std::env::split_paths`] for details. The first path which is successfully
+opened will be used.
This teaches rustc (and the autocxx macro) that all the different Rust bindings
for multiple different autocxx macros have been archived into this single file.
diff --git a/third_party/autocxx/gen/cmd/tests/cmd_test.rs b/third_party/autocxx/gen/cmd/tests/cmd_test.rs
index 7e455a2..6fd3382 100644
--- a/third_party/autocxx/gen/cmd/tests/cmd_test.rs
+++ b/third_party/autocxx/gen/cmd/tests/cmd_test.rs
@@ -147,6 +147,58 @@
}
#[test]
+fn test_gen_archive_first_entry() -> Result<(), Box<dyn std::error::Error>> {
+ let tmp_dir = tempdir()?;
+ base_test(&tmp_dir, RsGenMode::Archive, |_| {})?;
+ File::create(tmp_dir.path().join("cxx.h"))
+ .and_then(|mut cxx_h| cxx_h.write_all(autocxx_engine::HEADER.as_bytes()))?;
+ let r = build_from_folder(
+ tmp_dir.path(),
+ &tmp_dir.path().join("demo/main.rs"),
+ vec![tmp_dir.path().join("gen.rs.json")],
+ &["gen0.cc"],
+ RsFindMode::Custom(Box::new(|path: &Path| {
+ std::env::set_var(
+ "AUTOCXX_RS_JSON_ARCHIVE",
+ std::env::join_paths([&path.join("gen.rs.json"), Path::new("/nonexistent")])
+ .unwrap(),
+ )
+ })),
+ );
+ if KEEP_TEMPDIRS {
+ println!("Tempdir: {:?}", tmp_dir.into_path().to_str());
+ }
+ r.unwrap();
+ Ok(())
+}
+
+#[test]
+fn test_gen_archive_second_entry() -> Result<(), Box<dyn std::error::Error>> {
+ let tmp_dir = tempdir()?;
+ base_test(&tmp_dir, RsGenMode::Archive, |_| {})?;
+ File::create(tmp_dir.path().join("cxx.h"))
+ .and_then(|mut cxx_h| cxx_h.write_all(autocxx_engine::HEADER.as_bytes()))?;
+ let r = build_from_folder(
+ tmp_dir.path(),
+ &tmp_dir.path().join("demo/main.rs"),
+ vec![tmp_dir.path().join("gen.rs.json")],
+ &["gen0.cc"],
+ RsFindMode::Custom(Box::new(|path: &Path| {
+ std::env::set_var(
+ "AUTOCXX_RS_JSON_ARCHIVE",
+ std::env::join_paths([Path::new("/nonexistent"), &path.join("gen.rs.json")])
+ .unwrap(),
+ )
+ })),
+ );
+ if KEEP_TEMPDIRS {
+ println!("Tempdir: {:?}", tmp_dir.into_path().to_str());
+ }
+ r.unwrap();
+ Ok(())
+}
+
+#[test]
fn test_gen_multiple_in_archive() -> Result<(), Box<dyn std::error::Error>> {
let tmp_dir = tempdir()?;
diff --git a/third_party/autocxx/integration-tests/src/lib.rs b/third_party/autocxx/integration-tests/src/lib.rs
index 352c8ab..f4667d5 100644
--- a/third_party/autocxx/integration-tests/src/lib.rs
+++ b/third_party/autocxx/integration-tests/src/lib.rs
@@ -57,6 +57,9 @@
AutocxxRs,
AutocxxRsArchive,
AutocxxRsFile,
+ /// This just calls the callback instead of setting any environment variables. The callback
+ /// receives the path to the temporary directory.
+ Custom(Box<dyn FnOnce(&Path)>),
}
/// API to test building pre-generated files.
@@ -174,6 +177,7 @@
"AUTOCXX_RS_FILE",
self.temp_dir.path().join("gen0.include.rs"),
),
+ RsFindMode::Custom(f) => f(self.temp_dir.path()),
};
std::panic::catch_unwind(|| {
let test_cases = trybuild::TestCases::new();
diff --git a/third_party/autocxx/integration-tests/tests/integration_test.rs b/third_party/autocxx/integration-tests/tests/integration_test.rs
index 16c792b8..75319c9 100644
--- a/third_party/autocxx/integration-tests/tests/integration_test.rs
+++ b/third_party/autocxx/integration-tests/tests/integration_test.rs
@@ -9430,6 +9430,43 @@
}
#[test]
+fn test_abstract_up_multiple_bridge() {
+ let hdr = indoc! {"
+ #include <memory>
+ class A {
+ public:
+ virtual void foo() const = 0;
+ virtual ~A() {}
+ };
+ class B : public A {
+ public:
+ void foo() const {}
+ };
+ inline std::unique_ptr<A> get_a() { return std::make_unique<B>(); }
+ "};
+ let hexathorpe = Token);
+ let rs = quote! {
+ autocxx::include_cpp! {
+ #hexathorpe include "input.h"
+ safety!(unsafe_ffi)
+ generate!("A")
+ }
+ autocxx::include_cpp! {
+ #hexathorpe include "input.h"
+ safety!(unsafe_ffi)
+ name!(ffi2)
+ extern_cpp_type!("A", crate::ffi::A)
+ generate!("get_a")
+ }
+ fn main() {
+ let a = ffi2::get_a();
+ a.foo();
+ }
+ };
+ do_run_test_manual("", hdr, rs, None, None).unwrap();
+}
+
+#[test]
fn test_abstract_private() {
let hdr = indoc! {"
#include <memory>
diff --git a/third_party/autocxx/parser/src/file_locations.rs b/third_party/autocxx/parser/src/file_locations.rs
index a113d0f..6b3c58a 100644
--- a/third_party/autocxx/parser/src/file_locations.rs
+++ b/third_party/autocxx/parser/src/file_locations.rs
@@ -95,13 +95,13 @@
include!( #fname );
}
}
- FileLocationStrategy::FromAutocxxRsJsonArchive(fname) => {
- let archive = File::open(fname).unwrap_or_else(|_| panic!("Unable to open {}. This may mean you didn't run the codegen tool (autocxx_gen) before building the Rust code.", fname.to_string_lossy()));
+ FileLocationStrategy::FromAutocxxRsJsonArchive(fnames) => {
+ let archive = std::env::split_paths(fnames).flat_map(File::open).next().unwrap_or_else(|| panic!("Unable to open any of the paths listed in {}. This may mean you didn't run the codegen tool (autocxx_gen) before building the Rust code.", fnames.to_string_lossy()));
let multi_bindings: MultiBindings = serde_json::from_reader(archive)
.unwrap_or_else(|_| {
- panic!("Unable to interpret {} as JSON", fname.to_string_lossy())
+ panic!("Unable to interpret {} as JSON", fnames.to_string_lossy())
});
- multi_bindings.get(config).unwrap_or_else(|err| panic!("Unable to find a suitable set of bindings within the JSON archive {} ({}). This likely means that the codegen tool hasn't been rerun since some changes in your include_cpp! macro.", fname.to_string_lossy(), err))
+ multi_bindings.get(config).unwrap_or_else(|err| panic!("Unable to find a suitable set of bindings within the JSON archive {} ({}). This likely means that the codegen tool hasn't been rerun since some changes in your include_cpp! macro.", fnames.to_string_lossy(), err))
}
}
}