From df9f711468e4a9eb4a387acac8859003a7177bcf Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Mon, 15 May 2023 10:39:45 +0200 Subject: [PATCH] Define master_socket_name only once, and embed it with the request to fork. --- src/mc/api/RemoteApp.cpp | 20 +++++++++++--------- src/mc/remote/AppSide.cpp | 14 +++++--------- src/mc/remote/AppSide.hpp | 2 +- src/mc/remote/CheckerSide.cpp | 9 +++++---- src/mc/remote/CheckerSide.hpp | 2 +- src/mc/remote/mc_protocol.h | 7 +++++++ 6 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/mc/api/RemoteApp.cpp b/src/mc/api/RemoteApp.cpp index f28a1fa6e9..98278e6307 100644 --- a/src/mc/api/RemoteApp.cpp +++ b/src/mc/api/RemoteApp.cpp @@ -51,13 +51,11 @@ RemoteApp::RemoteApp(const std::vector& args, bool need_memory_introspect 0); xbt_assert(master_socket_ != -1, "Cannot create the master socket: %s", strerror(errno)); - struct sockaddr_un serv_addr = {}; - serv_addr.sun_family = AF_UNIX; - snprintf(serv_addr.sun_path, 64, "/tmp/simgrid-mc-%d", getpid()); - master_socket_name = serv_addr.sun_path; - auto addr_size = offsetof(struct sockaddr_un, sun_path) + strlen(serv_addr.sun_path); + master_socket_name = "/tmp/simgrid-mc-" + std::to_string(getpid()); + master_socket_name.resize(MC_SOCKET_NAME_LEN); // truncate socket name if it's too long + master_socket_name.back() = '\0'; // ensure the data are null-terminated #ifdef __linux__ - serv_addr.sun_path[0] = '\0'; // abstract socket, automatically removed after close + master_socket_name[0] = '\0'; // abstract socket, automatically removed after close #else unlink(master_socket_name.c_str()); // remove possible stale socket before bind atexit([]() { @@ -68,21 +66,25 @@ RemoteApp::RemoteApp(const std::vector& args, bool need_memory_introspect } #endif - xbt_assert(bind(master_socket_, (struct sockaddr*)&serv_addr, addr_size) >= 0, + struct sockaddr_un serv_addr = {}; + serv_addr.sun_family = AF_UNIX; + master_socket_name.copy(serv_addr.sun_path, MC_SOCKET_NAME_LEN); + + xbt_assert(bind(master_socket_, (struct sockaddr*)&serv_addr, sizeof serv_addr) >= 0, "Cannot bind the master socket to %c%s: %s.", (serv_addr.sun_path[0] ? serv_addr.sun_path[0] : '@'), serv_addr.sun_path + 1, strerror(errno)); xbt_assert(listen(master_socket_, SOMAXCONN) >= 0, "Cannot listen to the master socket: %s.", strerror(errno)); application_factory_ = std::make_unique(app_args_, need_memory_introspection); - checker_side_ = application_factory_->clone(master_socket_); + checker_side_ = application_factory_->clone(master_socket_, master_socket_name); } } void RemoteApp::restore_initial_state() { if (initial_snapshot_ == nullptr) // No memory introspection - checker_side_ = application_factory_->clone(master_socket_); + checker_side_ = application_factory_->clone(master_socket_, master_socket_name); #if SIMGRID_HAVE_STATEFUL_MC else initial_snapshot_->restore(*checker_side_->get_remote_memory()); diff --git a/src/mc/remote/AppSide.cpp b/src/mc/remote/AppSide.cpp index ac26914984..39326a0a22 100644 --- a/src/mc/remote/AppSide.cpp +++ b/src/mc/remote/AppSide.cpp @@ -152,7 +152,7 @@ void AppSide::handle_finalize(const s_mc_message_int_t* msg) const if (terminate_asap) ::_Exit(0); } -void AppSide::handle_fork(const s_mc_message_int_t* msg) +void AppSide::handle_fork(const s_mc_message_fork_t* msg) { int status; int pid; @@ -174,13 +174,9 @@ void AppSide::handle_fork(const s_mc_message_int_t* msg) struct sockaddr_un addr = {}; addr.sun_family = AF_UNIX; - snprintf(addr.sun_path, 64, "/tmp/simgrid-mc-%" PRIu64, msg->value); - auto addr_size = offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_path); -#ifdef __linux__ - addr.sun_path[0] = '\0'; // abstract socket -#endif + std::copy_n(begin(msg->socket_name), MC_SOCKET_NAME_LEN, addr.sun_path); - xbt_assert(connect(sock, (struct sockaddr*)&addr, addr_size) >= 0, "Cannot connect to Checker on %c%s: %s.", + xbt_assert(connect(sock, (struct sockaddr*)&addr, sizeof addr) >= 0, "Cannot connect to Checker on %c%s: %s.", (addr.sun_path[0] ? addr.sun_path[0] : '@'), addr.sun_path + 1, strerror(errno)); channel_.reset_socket(sock); @@ -330,8 +326,8 @@ void AppSide::handle_messages() break; case MessageType::FORK: - assert_msg_size("FORK", s_mc_message_int_t); - handle_fork((s_mc_message_int_t*)message_buffer.data()); + assert_msg_size("FORK", s_mc_message_fork_t); + handle_fork((s_mc_message_fork_t*)message_buffer.data()); break; case MessageType::WAIT_CHILD: diff --git a/src/mc/remote/AppSide.hpp b/src/mc/remote/AppSide.hpp index 03ed138509..3941ffae0c 100644 --- a/src/mc/remote/AppSide.hpp +++ b/src/mc/remote/AppSide.hpp @@ -34,7 +34,7 @@ private: void handle_deadlock_check(const s_mc_message_t* msg) const; void handle_simcall_execute(const s_mc_message_simcall_execute_t* message) const; void handle_finalize(const s_mc_message_int_t* msg) const; - void handle_fork(const s_mc_message_int_t* msg); + void handle_fork(const s_mc_message_fork_t* msg); void handle_wait_child(const s_mc_message_int_t* msg); void handle_need_meminfo(); void handle_actors_status() const; diff --git a/src/mc/remote/CheckerSide.cpp b/src/mc/remote/CheckerSide.cpp index 2b28f18d54..211995b856 100644 --- a/src/mc/remote/CheckerSide.cpp +++ b/src/mc/remote/CheckerSide.cpp @@ -258,11 +258,12 @@ CheckerSide::CheckerSide(int socket, CheckerSide* child_checker) wait_for_requests(); } -std::unique_ptr CheckerSide::clone(int master_socket) +std::unique_ptr CheckerSide::clone(int master_socket, const std::string& master_socket_name) { - s_mc_message_int_t m = {}; - m.type = MessageType::FORK; - m.value = getpid(); + s_mc_message_fork_t m = {}; + m.type = MessageType::FORK; + xbt_assert(master_socket_name.size() == MC_SOCKET_NAME_LEN); + std::copy_n(begin(master_socket_name), MC_SOCKET_NAME_LEN, begin(m.socket_name)); xbt_assert(get_channel().send(m) == 0, "Could not ask the app to fork on need."); int sock = accept(master_socket, nullptr /* I know who's connecting*/, nullptr); diff --git a/src/mc/remote/CheckerSide.hpp b/src/mc/remote/CheckerSide.hpp index 84c70d1252..d89f7121f6 100644 --- a/src/mc/remote/CheckerSide.hpp +++ b/src/mc/remote/CheckerSide.hpp @@ -57,7 +57,7 @@ public: void wait_for_requests(); /* Create a new CheckerSide by forking the currently existing one, and connect it through the master_socket */ - std::unique_ptr clone(int master_socket); + std::unique_ptr clone(int master_socket, const std::string& master_socket_name); /** Ask the application to run post-mortem analysis, and maybe to stop ASAP */ void finalize(bool terminate_asap = false); diff --git a/src/mc/remote/mc_protocol.h b/src/mc/remote/mc_protocol.h index 5da8aef608..e4cac55a47 100644 --- a/src/mc/remote/mc_protocol.h +++ b/src/mc/remote/mc_protocol.h @@ -19,6 +19,7 @@ #include #include +#include // ***** Messages namespace simgrid::mc { @@ -32,6 +33,7 @@ XBT_DECLARE_ENUM_CLASS(MessageType, NONE, NEED_MEMINFO, NEED_MEMINFO_REPLY, FORK } // namespace simgrid::mc constexpr unsigned MC_MESSAGE_LENGTH = 512; +constexpr unsigned MC_SOCKET_NAME_LEN = sizeof(sockaddr_un::sun_path); constexpr unsigned SIMCALL_SERIALIZATION_BUFFER_SIZE = 2048; /** Basic structure for a MC message @@ -87,6 +89,11 @@ struct s_mc_message_need_meminfo_reply_t { xbt_mheap_t mmalloc_default_mdp; }; +struct s_mc_message_fork_t { + simgrid::mc::MessageType type; + std::array socket_name; +}; + struct s_mc_message_simcall_execute_t { simgrid::mc::MessageType type; aid_t aid_; -- 2.20.1