rpm/python3-espressomd
SHA256
1
0
Fork 0
forked from pool/python3-espressomd
Code Pull Requests Activity

Accepting request 1143707 from home:jngrad:branches:devel:languages:python:numeric

Browse Source 
Fix undefined behavior

OBS-URL: https://build.opensuse.org/request/show/1143707
OBS-URL: https://build.opensuse.org/package/show/devel:languages:python:numeric/python3-espressomd?expand=0&rev=51
This commit is contained in:
Christoph Junghans 2024-02-02 17:29:05 +00:00 committed by Git OBS Bridge
parent 98dddf6f9d
commit 7ab71284ad
3 changed files with 489 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

479
mpi.patch Normal file
View File

@ -0,0 +1,479 @@
diff --git a/src/core/EspressoSystemStandAlone.cpp b/src/core/EspressoSystemStandAlone.cpp
index 90788c3c0..900fc5cdf 100644
--- a/src/core/EspressoSystemStandAlone.cpp
+++ b/src/core/EspressoSystemStandAlone.cpp
@@ -31,2 +31,3 @@
#include <boost/mpi.hpp>
+#include <boost/mpi/environment.hpp>
@@ -35,3 +36,3 @@
EspressoSystemStandAlone::EspressoSystemStandAlone(int argc, char **argv) {
- auto mpi_env = mpi_init(argc, argv);
+ m_mpi_env = mpi_init(argc, argv);
@@ -41,3 +42,3 @@ EspressoSystemStandAlone::EspressoSystemStandAlone(int argc, char **argv) {
// initialize the MpiCallbacks framework
- Communication::init(mpi_env);
+ Communication::init(m_mpi_env);
@@ -52,2 +53,6 @@ EspressoSystemStandAlone::EspressoSystemStandAlone(int argc, char **argv) {
+EspressoSystemStandAlone::~EspressoSystemStandAlone() {
+ Communication::deinit();
+}
+
void EspressoSystemStandAlone::set_box_l(Utils::Vector3d const &box_l) const {
diff --git a/src/core/EspressoSystemStandAlone.hpp b/src/core/EspressoSystemStandAlone.hpp
index d11838356..198b235c2 100644
--- a/src/core/EspressoSystemStandAlone.hpp
+++ b/src/core/EspressoSystemStandAlone.hpp
@@ -23,2 +23,10 @@
+#include <memory>
+
+namespace boost {
+namespace mpi {
+class environment;
+}
+} // namespace boost
+
/** Manager for a stand-alone ESPResSo system.
@@ -29,2 +37,3 @@ class EspressoSystemStandAlone {
EspressoSystemStandAlone(int argc, char **argv);
+ ~EspressoSystemStandAlone();
void set_box_l(Utils::Vector3d const &box_l) const;
@@ -36,2 +45,3 @@ class EspressoSystemStandAlone {
bool head_node;
+ std::shared_ptr<boost::mpi::environment> m_mpi_env;
};
diff --git a/src/core/MpiCallbacks.hpp b/src/core/MpiCallbacks.hpp
index 2f58f460d..153c410f6 100644
--- a/src/core/MpiCallbacks.hpp
+++ b/src/core/MpiCallbacks.hpp
@@ -50,2 +50,3 @@
#include <boost/mpi/communicator.hpp>
+#include <boost/mpi/environment.hpp>
#include <boost/optional.hpp>
@@ -366,3 +367,3 @@ class MpiCallbacks {
std::tuple<Args...>>::value>>
- CallbackHandle(MpiCallbacks *cb, F &&f)
+ CallbackHandle(std::shared_ptr<MpiCallbacks> cb, F &&f)
: m_id(cb->add(std::forward<F>(f))), m_cb(cb) {}
@@ -376,3 +377,3 @@ class MpiCallbacks {
int m_id;
- MpiCallbacks *m_cb;
+ std::shared_ptr<MpiCallbacks> m_cb;
@@ -402,3 +403,2 @@ class MpiCallbacks {
- MpiCallbacks *cb() const { return m_cb; }
int id() const { return m_id; }
@@ -421,4 +421,6 @@ class MpiCallbacks {
explicit MpiCallbacks(boost::mpi::communicator comm,
+ std::shared_ptr<boost::mpi::environment> mpi_env,
bool abort_on_exit = true)
- : m_abort_on_exit(abort_on_exit), m_comm(std::move(comm)) {
+ : m_abort_on_exit(abort_on_exit), m_comm(std::move(comm)),
+ m_mpi_env(std::move(mpi_env)) {
/* Add a dummy at id 0 for loop abort. */
@@ -740,2 +742,7 @@ class MpiCallbacks {
+ /**
+ * The MPI environment used for the callbacks.
+ */
+ std::shared_ptr<boost::mpi::environment> m_mpi_env;
+
/**
diff --git a/src/core/communication.cpp b/src/core/communication.cpp
index 20b7d0526..d8b26cb38 100644
--- a/src/core/communication.cpp
+++ b/src/core/communication.cpp
@@ -30,2 +30,4 @@
#include <boost/mpi.hpp>
+#include <boost/mpi/communicator.hpp>
+#include <boost/mpi/environment.hpp>
@@ -41,7 +43,2 @@
-namespace Communication {
-auto const &mpi_datatype_cache = boost::mpi::detail::mpi_datatype_cache();
-std::shared_ptr<boost::mpi::environment> mpi_env;
-} // namespace Communication
-
boost::mpi::communicator comm_cart;
@@ -49,3 +46,3 @@ boost::mpi::communicator comm_cart;
namespace Communication {
-std::unique_ptr<MpiCallbacks> m_callbacks;
+static std::shared_ptr<MpiCallbacks> m_callbacks;
@@ -57,2 +54,8 @@ MpiCallbacks &mpiCallbacks() {
}
+
+std::shared_ptr<MpiCallbacks> mpiCallbacksHandle() {
+ assert(m_callbacks && "Mpi not initialized!");
+
+ return m_callbacks;
+}
} // namespace Communication
@@ -122,4 +125,2 @@ namespace Communication {
void init(std::shared_ptr<boost::mpi::environment> mpi_env) {
- Communication::mpi_env = std::move(mpi_env);
-
MPI_Comm_size(MPI_COMM_WORLD, &n_nodes);
@@ -133,3 +134,3 @@ void init(std::shared_ptr<boost::mpi::environment> mpi_env) {
Communication::m_callbacks =
- std::make_unique<Communication::MpiCallbacks>(comm_cart);
+ std::make_shared<Communication::MpiCallbacks>(comm_cart, mpi_env);
@@ -139,2 +140,4 @@ void init(std::shared_ptr<boost::mpi::environment> mpi_env) {
}
+
+void deinit() { Communication::m_callbacks.reset(); }
} // namespace Communication
diff --git a/src/core/communication.hpp b/src/core/communication.hpp
index cab2d8507..243bf8d05 100644
--- a/src/core/communication.hpp
+++ b/src/core/communication.hpp
@@ -65,2 +65,3 @@ namespace Communication {
MpiCallbacks &mpiCallbacks();
+std::shared_ptr<MpiCallbacks> mpiCallbacksHandle();
} // namespace Communication
@@ -138,6 +139,2 @@ namespace Communication {
*
- * and calls @ref on_program_start. Keeps a copy of
- * the pointer to the mpi environment to keep it alive
- * while the program is loaded.
- *
* @param mpi_env MPI environment that should be used
@@ -145,3 +142,13 @@ namespace Communication {
void init(std::shared_ptr<boost::mpi::environment> mpi_env);
+void deinit();
} // namespace Communication
+
+struct MpiContainerUnitTest {
+ std::shared_ptr<boost::mpi::environment> m_mpi_env;
+ MpiContainerUnitTest(int argc, char **argv) {
+ m_mpi_env = mpi_init(argc, argv);
+ Communication::init(m_mpi_env);
+ }
+ ~MpiContainerUnitTest() { Communication::deinit(); }
+};
#endif
diff --git a/src/core/unit_tests/EspressoSystemStandAlone_test.cpp b/src/core/unit_tests/EspressoSystemStandAlone_test.cpp
index aec1c71d5..d1fbe0cca 100644
--- a/src/core/unit_tests/EspressoSystemStandAlone_test.cpp
+++ b/src/core/unit_tests/EspressoSystemStandAlone_test.cpp
@@ -359,3 +359,5 @@ int main(int argc, char **argv) {
- return boost::unit_test::unit_test_main(init_unit_test, argc, argv);
+ int retval = boost::unit_test::unit_test_main(init_unit_test, argc, argv);
+ espresso::system.reset();
+ return retval;
}
diff --git a/src/core/unit_tests/MpiCallbacks_test.cpp b/src/core/unit_tests/MpiCallbacks_test.cpp
index 3afbe3f2a..7ae484aa8 100644
--- a/src/core/unit_tests/MpiCallbacks_test.cpp
+++ b/src/core/unit_tests/MpiCallbacks_test.cpp
@@ -31,2 +31,3 @@
#include <boost/mpi.hpp>
+#include <boost/mpi/environment.hpp>
#include <boost/optional.hpp>
@@ -38,2 +39,3 @@
+static std::weak_ptr<boost::mpi::environment> mpi_env;
static bool called = false;
@@ -113,3 +115,3 @@ BOOST_AUTO_TEST_CASE(adding_function_ptr_cb) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cb(world);
+ Communication::MpiCallbacks cb(world, ::mpi_env.lock());
@@ -145,3 +147,3 @@ BOOST_AUTO_TEST_CASE(RegisterCallback) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cb(world);
+ Communication::MpiCallbacks cb(world, ::mpi_env.lock());
@@ -159,3 +161,4 @@ BOOST_AUTO_TEST_CASE(CallbackHandle) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ auto const cbs =
+ std::make_shared<Communication::MpiCallbacks>(world, ::mpi_env.lock());
@@ -163,3 +166,3 @@ BOOST_AUTO_TEST_CASE(CallbackHandle) {
Communication::CallbackHandle<std::string> cb(
- &cbs, [&m_called](std::string s) {
+ cbs, [&m_called](std::string s) {
BOOST_CHECK_EQUAL("CallbackHandle", s);
@@ -172,3 +175,3 @@ BOOST_AUTO_TEST_CASE(CallbackHandle) {
} else {
- cbs.loop();
+ cbs->loop();
BOOST_CHECK(called);
@@ -184,3 +187,3 @@ BOOST_AUTO_TEST_CASE(reduce_callback) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -205,3 +208,3 @@ BOOST_AUTO_TEST_CASE(ignore_callback) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -231,3 +234,3 @@ BOOST_AUTO_TEST_CASE(one_rank_callback) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -256,3 +259,3 @@ BOOST_AUTO_TEST_CASE(main_rank_callback) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -275,3 +278,3 @@ BOOST_AUTO_TEST_CASE(call_all) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -296,3 +299,3 @@ BOOST_AUTO_TEST_CASE(check_exceptions) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cbs(world);
+ Communication::MpiCallbacks cbs(world, ::mpi_env.lock());
@@ -308,3 +311,4 @@ BOOST_AUTO_TEST_CASE(check_exceptions) {
int main(int argc, char **argv) {
- boost::mpi::environment mpi_env(argc, argv);
+ auto const mpi_env = std::make_shared<boost::mpi::environment>(argc, argv);
+ ::mpi_env = mpi_env;
diff --git a/src/core/unit_tests/Verlet_list_test.cpp b/src/core/unit_tests/Verlet_list_test.cpp
index 433683965..43947c50c 100644
--- a/src/core/unit_tests/Verlet_list_test.cpp
+++ b/src/core/unit_tests/Verlet_list_test.cpp
@@ -243,2 +243,3 @@ int main(int argc, char **argv) {
}
+ espresso::system.reset();
return error_code;
diff --git a/src/python/espressomd/_init.pyx b/src/python/espressomd/_init.pyx
index 0ef6ee52f..0b9699914 100644
--- a/src/python/espressomd/_init.pyx
+++ b/src/python/espressomd/_init.pyx
@@ -19,2 +19,3 @@
import sys
+import atexit
from . cimport script_interface
@@ -30,3 +31,13 @@ communication.init(mpi_env)
# Has to be _after_ mpi_init
-script_interface.init(communication.mpiCallbacks())
+script_interface.init(communication.mpiCallbacksHandle())
+
+
+def session_shutdown():
+ mpi_env.reset()
+ communication.deinit()
+ script_interface.deinit()
+
+
+atexit.register(session_shutdown)
+
diff --git a/src/python/espressomd/communication.pxd b/src/python/espressomd/communication.pxd
index aeb4f43ad..7065f96a9 100644
--- a/src/python/espressomd/communication.pxd
+++ b/src/python/espressomd/communication.pxd
@@ -32,2 +32,4 @@ cdef extern from "communication.hpp" namespace "Communication":
MpiCallbacks & mpiCallbacks()
+ shared_ptr[MpiCallbacks] mpiCallbacksHandle()
void init(shared_ptr[environment])
+ void deinit()
diff --git a/src/python/espressomd/script_interface.pxd b/src/python/espressomd/script_interface.pxd
index bd2b0c286..5e9bd38dd 100644
--- a/src/python/espressomd/script_interface.pxd
+++ b/src/python/espressomd/script_interface.pxd
@@ -60,3 +60,3 @@ cdef extern from "script_interface/ContextManager.hpp" namespace "ScriptInterfac
cdef cppclass ContextManager:
- ContextManager(MpiCallbacks & , const Factory[ObjectHandle] & )
+ ContextManager(const shared_ptr[MpiCallbacks] & , const Factory[ObjectHandle] & )
shared_ptr[ObjectHandle] make_shared(CreationPolicy, const string &, const VariantMap) except +
@@ -71,2 +71,3 @@ cdef extern from "script_interface/get_value.hpp" namespace "ScriptInterface":
-cdef void init(MpiCallbacks &)
+cdef void init(const shared_ptr[MpiCallbacks] &)
+cdef void deinit()
diff --git a/src/python/espressomd/script_interface.pyx b/src/python/espressomd/script_interface.pyx
index d6fff6879..01922140d 100644
--- a/src/python/espressomd/script_interface.pyx
+++ b/src/python/espressomd/script_interface.pyx
@@ -486,3 +486,3 @@ def script_interface_register(c):
-cdef void init(MpiCallbacks & cb):
+cdef void init(const shared_ptr[MpiCallbacks] &cb):
cdef Factory[ObjectHandle] f
@@ -493 +493,6 @@ cdef void init(MpiCallbacks & cb):
_om = make_shared[ContextManager](cb, f)
+
+
+cdef void deinit():
+ global _om
+ _om.reset()
diff --git a/src/python/espressomd/system.pyx b/src/python/espressomd/system.pyx
index 3bb08f381..3003f66c9 100644
--- a/src/python/espressomd/system.pyx
+++ b/src/python/espressomd/system.pyx
@@ -204,2 +204,4 @@ cdef class System:
+ self._setup_atexit()
+
# lock class
@@ -208,2 +210,10 @@ cdef class System:
+ def _setup_atexit(self):
+ import atexit
+
+ def session_shutdown():
+ self.actors.clear()
+
+ atexit.register(session_shutdown)
+
# __getstate__ and __setstate__ define the pickle interaction
@@ -232,2 +242,3 @@ cdef class System:
System.__setattr__(self, property_name, params[property_name])
+ self._setup_atexit()
diff --git a/src/script_interface/ContextManager.cpp b/src/script_interface/ContextManager.cpp
index 7bb3dfd37..e7f20c347 100644
--- a/src/script_interface/ContextManager.cpp
+++ b/src/script_interface/ContextManager.cpp
@@ -55,6 +55,7 @@ std::string ContextManager::serialize(const ObjectHandle *o) const {
-ContextManager::ContextManager(Communication::MpiCallbacks &callbacks,
- const Utils::Factory<ObjectHandle> &factory) {
+ContextManager::ContextManager(
+ std::shared_ptr<Communication::MpiCallbacks> const &callbacks,
+ const Utils::Factory<ObjectHandle> &factory) {
auto local_context =
- std::make_shared<LocalContext>(factory, callbacks.comm());
+ std::make_shared<LocalContext>(factory, callbacks->comm());
@@ -63,3 +64,3 @@ ContextManager::ContextManager(Communication::MpiCallbacks &callbacks,
m_global_context =
- (callbacks.comm().size() > 1)
+ (callbacks->comm().size() > 1)
? std::make_shared<GlobalContext>(callbacks, local_context)
diff --git a/src/script_interface/ContextManager.hpp b/src/script_interface/ContextManager.hpp
index 7f5a098a5..47fa10a73 100644
--- a/src/script_interface/ContextManager.hpp
+++ b/src/script_interface/ContextManager.hpp
@@ -69,3 +69,3 @@ class ContextManager {
- ContextManager(Communication::MpiCallbacks &callbacks,
+ ContextManager(std::shared_ptr<Communication::MpiCallbacks> const &callbacks,
const Utils::Factory<ObjectHandle> &factory);
diff --git a/src/script_interface/GlobalContext.hpp b/src/script_interface/GlobalContext.hpp
index d4de6ebbc..ede913359 100644
--- a/src/script_interface/GlobalContext.hpp
+++ b/src/script_interface/GlobalContext.hpp
@@ -88,9 +88,9 @@ class GlobalContext : public Context {
public:
- GlobalContext(Communication::MpiCallbacks &callbacks,
+ GlobalContext(std::shared_ptr<Communication::MpiCallbacks> const &callbacks,
std::shared_ptr<LocalContext> node_local_context)
: m_local_objects(), m_node_local_context(std::move(node_local_context)),
- m_is_head_node(callbacks.comm().rank() == 0),
+ m_is_head_node(callbacks->comm().rank() == 0),
// NOLINTNEXTLINE(bugprone-throw-keyword-missing)
- m_parallel_exception_handler(callbacks.comm()),
- cb_make_handle(&callbacks,
+ m_parallel_exception_handler(callbacks->comm()),
+ cb_make_handle(callbacks,
[this](ObjectId id, const std::string &name,
@@ -99,3 +99,3 @@ class GlobalContext : public Context {
}),
- cb_set_parameter(&callbacks,
+ cb_set_parameter(callbacks,
[this](ObjectId id, std::string const &name,
@@ -104,3 +104,3 @@ class GlobalContext : public Context {
}),
- cb_call_method(&callbacks,
+ cb_call_method(callbacks,
[this](ObjectId id, std::string const &name,
@@ -109,3 +109,3 @@ class GlobalContext : public Context {
}),
- cb_delete_handle(&callbacks,
+ cb_delete_handle(callbacks,
[this](ObjectId id) { delete_handle(id); }) {}
diff --git a/src/script_interface/tests/GlobalContext_test.cpp b/src/script_interface/tests/GlobalContext_test.cpp
index b5556b2d1..da0401ad4 100644
--- a/src/script_interface/tests/GlobalContext_test.cpp
+++ b/src/script_interface/tests/GlobalContext_test.cpp
@@ -28,2 +28,3 @@
#include <boost/mpi/communicator.hpp>
+#include <boost/mpi/environment.hpp>
@@ -34,2 +35,4 @@
+static std::weak_ptr<boost::mpi::environment> mpi_env;
+
namespace si = ScriptInterface;
@@ -56,9 +59,8 @@ struct Dummy : si::ObjectHandle {
-auto make_global_context(Communication::MpiCallbacks &cb) {
+auto make_global_context(std::shared_ptr<Communication::MpiCallbacks> &cb) {
Utils::Factory<si::ObjectHandle> factory;
factory.register_new<Dummy>("Dummy");
- boost::mpi::communicator comm;
return std::make_shared<si::GlobalContext>(
- cb, std::make_shared<si::LocalContext>(factory, comm));
+ cb, std::make_shared<si::LocalContext>(factory, cb->comm()));
}
@@ -67,3 +69,4 @@ BOOST_AUTO_TEST_CASE(GlobalContext_make_shared) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cb{world};
+ auto cb =
+ std::make_shared<Communication::MpiCallbacks>(world, ::mpi_env.lock());
auto ctx = make_global_context(cb);
@@ -76,3 +79,3 @@ BOOST_AUTO_TEST_CASE(GlobalContext_make_shared) {
} else {
- cb.loop();
+ cb->loop();
}
@@ -82,3 +85,4 @@ BOOST_AUTO_TEST_CASE(GlobalContext_serialization) {
boost::mpi::communicator world;
- Communication::MpiCallbacks cb{world};
+ auto cb =
+ std::make_shared<Communication::MpiCallbacks>(world, ::mpi_env.lock());
auto ctx = make_global_context(cb);
@@ -110,3 +114,3 @@ BOOST_AUTO_TEST_CASE(GlobalContext_serialization) {
} else {
- cb.loop();
+ cb->loop();
}
@@ -115,3 +119,4 @@ BOOST_AUTO_TEST_CASE(GlobalContext_serialization) {
int main(int argc, char **argv) {
- boost::mpi::environment mpi_env(argc, argv);
+ auto const mpi_env = std::make_shared<boost::mpi::environment>(argc, argv);
+ ::mpi_env = mpi_env;
diff --git a/src/script_interface/tests/ParallelExceptionHandler_test.cpp b/src/script_interface/tests/ParallelExceptionHandler_test.cpp
index c78c11c0c..30f0bb7dc 100644
--- a/src/script_interface/tests/ParallelExceptionHandler_test.cpp
+++ b/src/script_interface/tests/ParallelExceptionHandler_test.cpp
@@ -47,2 +47,3 @@
#include <boost/mpi/communicator.hpp>
+#include <boost/mpi/environment.hpp>
@@ -54,2 +55,4 @@ namespace utf = boost::unit_test;
+static std::weak_ptr<boost::mpi::environment> mpi_env;
+
namespace Testing {
@@ -70,3 +73,3 @@ BOOST_AUTO_TEST_CASE(parallel_exceptions) {
boost::mpi::communicator world;
- Communication::MpiCallbacks callbacks{world};
+ Communication::MpiCallbacks callbacks{world, ::mpi_env.lock()};
ErrorHandling::init_error_handling(callbacks);
@@ -135,3 +138,4 @@ BOOST_AUTO_TEST_CASE(parallel_exceptions) {
int main(int argc, char **argv) {
- boost::mpi::environment mpi_env(argc, argv);
+ auto const mpi_env = std::make_shared<boost::mpi::environment>(argc, argv);
+ ::mpi_env = mpi_env;

6
python3-espressomd.changes
View File

@ -1,3 +1,9 @@
-------------------------------------------------------------------
Fri Feb 2 16:23:08 UTC 2024 - Jean-Noel Grad <jgrad@icp.uni-stuttgart.de>
- Fix undefined behavior due to improper handling of MPI static globals
- Add mpi.patch to remove MPI globals (gh#espressomd/espresso#4858)
-------------------------------------------------------------------
Mon Oct 2 12:11:53 UTC 2023 - Ondřej Súkup <mimi.vx@gmail.com>

6
python3-espressomd.spec
View File

@ -1,7 +1,7 @@
#
# spec file
# spec file for package python3-espressomd
#
# Copyright (c) 2023 SUSE LLC
# Copyright (c) 2024 SUSE LLC
# Copyright (c) 2014 Christoph Junghans
#
# All modifications and additions to the file contributed by third parties
@ -34,6 +34,8 @@ Patch0: setuptools.patch
Patch1: tracers.patch
# PATCH-FIX-UPSTREAM array-bounds.patch gh#espressomd/espresso#4715
Patch2: array-bounds.patch
# PATCH-FIX-UPSTREAM mpi.patch gh#espressomd/espresso#4858
Patch3: mpi.patch
# According to gh#espressomd/espresso#4537 32bit architectures are not supported any more
ExcludeArch: %{ix86}
BuildRequires: cmake