Better handling of errors during dynamic catalogue loading (#1702)

* Add platform abstraction for loading DLL/SOs.
* Throw more informative exceptions when handling DLL/SOs.
* More informative errors when dealing with dynamically loaded catalogues.
* Translate `arb::file_not_found_error` to `FileNotFoundError` in Python.

CMakeLists.txt

@@ -186,7 +186,13 @@ install(TARGETS arbor-config-defs EXPORT arbor-targets)
 # Interface library `arbor-private-deps` collects dependencies, options etc.
 # for the arbor library.
 add_library(arbor-private-deps INTERFACE)
-target_link_libraries(arbor-private-deps INTERFACE arbor-config-defs ext-random123 ${CMAKE_DL_LIBS})
+target_link_libraries(arbor-private-deps INTERFACE arbor-config-defs ext-random123)
+if (UNIX)
+    target_compile_definitions(arbor-config-defs INTERFACE ARB_HAVE_DL)
+    target_link_libraries(arbor-private-deps INTERFACE ${CMAKE_DL_LIBS})
+else()
+    message(FATAL_ERROR "No support for dynamic loading on current platform.")
+endif ()
 install(TARGETS arbor-private-deps EXPORT arbor-targets)
 
 # Interface library `arborenv-private-deps` collects dependencies, options etc.

arbor/arbexcept.cpp

@@ -124,14 +124,16 @@ range_check_failure::range_check_failure(const std::string& whatstr, double valu
 {}
 
 file_not_found_error::file_not_found_error(const std::string &fn)
-    : arbor_exception(pprintf("Could not find file '{}'", fn)),
+    : arbor_exception(pprintf("Could not find readable file at '{}'", fn)),
       filename{fn}
 {}
 
-bad_catalogue_error::bad_catalogue_error(const std::string &fn, const std::string& call)
-    : arbor_exception(pprintf("Error in '{}' while opening catalogue '{}'", call, fn)),
-      filename{fn},
-      failed_call{call}
+bad_catalogue_error::bad_catalogue_error(const std::string& msg)
+    : arbor_exception(pprintf("Error while opening catalogue '{}'", msg))
+{}
+
+bad_catalogue_error::bad_catalogue_error(const std::string& msg, const std::any& pe)
+    : arbor_exception(pprintf("Error while opening catalogue '{}'", msg)), platform_error(pe)
 {}
 
 unsupported_abi_error::unsupported_abi_error(size_t v):

arbor/include/arbor/arbexcept.hpp

@@ -146,10 +146,11 @@ struct file_not_found_error: arbor_exception {
     std::string filename;
 };
 
+//
 struct bad_catalogue_error: arbor_exception {
-    bad_catalogue_error(const std::string& fn, const std::string& call);
-    std::string filename;
-    std::string failed_call;
+    bad_catalogue_error(const std::string&);
+    bad_catalogue_error(const std::string&, const std::any&);
+    std::any platform_error;
 };
 
 // ABI errors

arbor/mechcat.cpp

@@ -5,8 +5,6 @@
 #include <vector>
 #include <cassert>
 
-#include <dlfcn.h>
-
 #include <arbor/version.hpp>
 #include <arbor/arbexcept.hpp>
 #include <arbor/mechcat.hpp>
@@ -17,6 +15,7 @@
 #include "util/rangeutil.hpp"
 #include "util/maputil.hpp"
 #include "util/span.hpp"
+#include "util/dl.hpp"
 
 /* Notes on implementation:
  *
@@ -584,21 +583,18 @@ std::pair<mechanism_ptr, mechanism_overrides> mechanism_catalogue::instance_impl
 
 mechanism_catalogue::~mechanism_catalogue() = default;
 
-static void check_dlerror(const std::string& fn, const std::string& call) {
-    auto error = dlerror();
-    if (error) { throw arb::bad_catalogue_error{fn, call}; }
-}
-
 const mechanism_catalogue& load_catalogue(const std::string& fn) {
     typedef const void* global_catalogue_t();
-
-    auto plugin = dlopen(fn.c_str(), RTLD_LAZY);
-    check_dlerror(fn, "dlopen");
-    assert(plugin);
-
-    auto get_catalogue = (global_catalogue_t*)dlsym(plugin, "get_catalogue");
-    check_dlerror(fn, "dlsym");
-
+    global_catalogue_t* get_catalogue = nullptr;
+    try {
+        auto plugin = dl_open(fn);
+        get_catalogue = dl_get_symbol<global_catalogue_t*>(plugin, "get_catalogue");
+    } catch(dl_error& e) {
+        throw bad_catalogue_error{e.what(), {e}};
+    }
+    if (!get_catalogue) {
+        throw bad_catalogue_error{util::pprintf("Unusable symbol 'get_catalogue' in shared object '{}'", fn)};
+    }
     /* NOTE We do not free the DSO handle here and accept retaining the handles
        until termination since the mechanisms provided by the catalogue may have
        a different lifetime than the actual catalogue itfself. This is not a

arbor/util/dl.hpp

@@ -0,0 +1,5 @@
+#pragma once
+
+#ifdef ARB_HAVE_DL
+#include "util/dl_platform_posix.hpp"
+#endif

arbor/util/dl_platform_posix.hpp

@@ -0,0 +1,52 @@
+#pragma once
+
+#include <fstream>
+#include <string>
+
+#include <dlfcn.h>
+
+#include <arbor/arbexcept.hpp>
+
+#include "util/strprintf.hpp"
+
+namespace arb {
+
+struct dl_error: arbor_exception {
+    dl_error(const std::string& msg): arbor_exception{msg} {}
+};
+
+struct dl_handle {
+    void* dl = nullptr;
+};
+
+inline
+dl_handle dl_open(const std::string& fn) {
+    // Test if we can open file
+    if (!std::ifstream{fn.c_str()}.good()) throw file_not_found_error{fn};
+    // Call once to clear errors not caused by us
+    dlerror();
+    // Try to open shared object
+    auto result = dlopen(fn.c_str(), RTLD_LAZY);
+    // dlopen fails by returning NULL
+    if (!result) throw dl_error{util::pprintf("[POSIX] dl_open failed with: {}", dlerror())};
+    return {result};
+}
+
+template<typename T> inline
+T dl_get_symbol(const dl_handle& handle, const std::string& symbol) {
+    // Call once to clear errors not caused by us
+    dlerror();
+    // Get symbol from shared object, may return NULL if that is what symbol refers to
+    auto result = dlsym(handle.dl, symbol.c_str());
+    // ... so we can only ask for dlerror here
+    if (auto error = dlerror()) throw dl_error{util::pprintf("[POSIX] dl_get_symbol failed with: {}", error)};
+    return reinterpret_cast<T>(result);
+}
+
+inline
+void dl_close(dl_handle& handle) {
+    dlclose(handle.dl);
+    handle.dl = nullptr;
+}
+
+}

python/pyarb.cpp

@@ -3,6 +3,7 @@
 
 #include <arbor/spike.hpp>
 #include <arbor/common_types.hpp>
+#include <arbor/arbexcept.hpp>
 #include <arbor/version.hpp>
 
 #include "pyarb.hpp"
@@ -43,6 +44,9 @@ PYBIND11_MODULE(_arbor, m) {
     m.doc() = "arbor: multicompartment neural network models.";
     m.attr("__version__") = ARB_VERSION;
 
+    // Translate Arbor errors -> Python exceptions.
+    pybind11::register_exception<arb::file_not_found_error>(m, "FileNotFoundError", PyExc_FileNotFoundError);
+
     pyarb::register_cable_loader(m);
     pyarb::register_cable_probes(m, global_ptr);
     pyarb::register_cells(m);

python/test/unit/test_catalogues.py

@@ -48,7 +48,7 @@ def cell_description(self, gid):
 
 class Catalogues(unittest.TestCase):
     def test_nonexistent(self):
-        with self.assertRaises(RuntimeError):
+        with self.assertRaises(FileNotFoundError):
             arb.load_catalogue("_NO_EXIST_.so")
 
     def test_shared_catalogue(self):

test/unit/test_mechcat.cpp

@@ -280,7 +280,7 @@ TEST(mechcat, names) {
 
 #ifdef USE_DYNAMIC_CATALOGUES
 TEST(mechcat, loading) {
-    EXPECT_THROW(load_catalogue(LIBDIR "/does-not-exist-catalogue.so"), bad_catalogue_error);
+    EXPECT_THROW(load_catalogue(LIBDIR "/does-not-exist-catalogue.so"), file_not_found_error);
     EXPECT_THROW(load_catalogue(LIBDIR "/libarbor.a"), bad_catalogue_error);
     const mechanism_catalogue* cat = nullptr;
     EXPECT_NO_THROW(cat = &load_catalogue(LIBDIR "/dummy-catalogue.so"));