diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp
index 7f5aca86d..50f066057 100644
--- a/src/query/plan/operator.cpp
+++ b/src/query/plan/operator.cpp
@@ -3735,39 +3735,6 @@ std::vector<Symbol> CallProcedure::ModifiedSymbols(
namespace {
-// Return true if we handled one of the special `mg` module procedures for
-// reloading query modules.
-// @throw QueryRuntimeException in case of error during procedure invocation.
-bool HandleReloadProcedures(
- const std::string_view &fully_qualified_procedure_name,
- const std::vector<Expression *> &args, ExpressionEvaluator *evaluator) {
- // It would be great to simply register `reload_all_modules` as a
- // regular procedure on a `mg` module, so we don't have a special case here.
- // Unfortunately, reloading requires taking a write lock, and we would
- // acquire a read lock by getting the module.
- if (fully_qualified_procedure_name == "mg.reload_all_modules") {
- if (!args.empty())
- throw QueryRuntimeException(
- "'mg.reload_all_modules' requires no arguments.");
- procedure::gModuleRegistry.ReloadAllModules();
- return true;
- } else if (fully_qualified_procedure_name == "mg.reload") {
- // This is a special case for the same reasons as `mg.reload_all_modules`.
- if (args.size() != 1U)
- throw QueryRuntimeException("'mg.reload' requires exactly 1 argument.");
- const auto &arg = args.front()->Accept(*evaluator);
- if (!arg.IsString()) {
- throw QueryRuntimeException(
- "'mg.reload' argument named 'module_name' at position 0 must be of "
- "type STRING.");
- }
- const auto &module_name = arg.ValueString();
- procedure::gModuleRegistry.ReloadModuleNamed(module_name);
- return true;
- }
- return false;
-}
-
// Return the ModulePtr and `mgp_proc *` of the found procedure after resolving
// `fully_qualified_procedure_name`. `memory` is used for temporary allocations
// inside this function. ModulePtr must be kept alive to make sure it won't be
@@ -3906,15 +3873,13 @@ class CallProcedureCursor : public Cursor {
ExpressionEvaluator evaluator(&frame, context.symbol_table,
context.evaluation_context,
context.db_accessor, graph_view);
- // First try to handle special procedures for (re)loading modules.
- if (HandleReloadProcedures(self_->procedure_name_, self_->arguments_,
- &evaluator))
- continue;
- // Nothing special, so find the regular procedure and invoke it.
// It might be a good idea to resolve the procedure name once, at the
// start. Unfortunately, this could deadlock if we tried to invoke a
// procedure from a module (read lock) and reload a module (write lock)
- // inside the same execution thread.
+ // inside the same execution thread. Also, our RWLock is setup so that
+ // it's not possible for a single thread to request multiple read locks.
+ // Builtin module registration in query/procedure/module.cpp depends on
+ // this locking scheme.
const auto &[module, proc] = FindProcedureOrThrow(
self_->procedure_name_, context.evaluation_context.memory);
result_.signature = &proc->results;
diff --git a/src/query/procedure/module.cpp b/src/query/procedure/module.cpp
index a1334d6c3..12ad1b09f 100644
--- a/src/query/procedure/module.cpp
+++ b/src/query/procedure/module.cpp
@@ -74,8 +74,64 @@ bool CloseModule(Module *module) {
return true;
}
+// Return true if the module is builtin, i.e. not loaded from dynamic lib.
+// Builtin modules cannot be reloaded nor unloaded.
bool IsBuiltinModule(const Module &module) { return module.handle == nullptr; }
+void RegisterMgReload(ModuleRegistry *module_registry, utils::RWLock *lock,
+ Module *module) {
+ // Reloading relies on the fact that regular procedure invocation through
+ // CallProcedureCursor::Pull takes ModuleRegistry::lock_ with READ access. To
+ // reload modules we have to upgrade our READ access to WRITE access,
+ // therefore we release the READ lock and invoke the reload function which
+ // takes the WRITE lock. Obviously, some other thread may take a READ or WRITE
+ // lock during our transition when we hold no such lock. In this case it is
+ // fine, because our builtin module cannot be unloaded and we are ok with
+ // using the new state of module_registry when we manage to acquire the lock
+ // we desire. Note, deadlock between threads should not be possible, because a
+ // single thread may only take either a READ or a WRITE lock, it's not
+ // possible for a thread to hold both. If a thread tries to do that, it will
+ // deadlock immediately (no other thread needs to do anything).
+ auto with_unlock_shared = [lock](const auto &reload_function) {
+ lock->unlock_shared();
+ try {
+ reload_function();
+ // There's no finally in C++, but we have to return our original READ lock
+ // state in any possible case.
+ } catch (...) {
+ lock->lock_shared();
+ throw;
+ }
+ lock->lock_shared();
+ };
+ auto reload_all_cb = [module_registry, with_unlock_shared](
+ const mgp_list *, const mgp_graph *, mgp_result *res,
+ mgp_memory *) {
+ bool succ = false;
+ with_unlock_shared([&]() { succ = module_registry->ReloadAllModules(); });
+ if (!succ) mgp_result_set_error_msg(res, "Failed to reload all modules.");
+ };
+ mgp_proc reload_all("reload_all", reload_all_cb, utils::NewDeleteResource());
+ module->procedures.emplace("reload_all", std::move(reload_all));
+ auto reload_cb = [module_registry, with_unlock_shared](
+ const mgp_list *args, const mgp_graph *, mgp_result *res,
+ mgp_memory *) {
+ CHECK(mgp_list_size(args) == 1U) << "Should have been type checked already";
+ const mgp_value *arg = mgp_list_at(args, 0);
+ CHECK(mgp_value_is_string(arg)) << "Should have been type checked already";
+ bool succ = false;
+ with_unlock_shared([&]() {
+ succ = module_registry->ReloadModuleNamed(mgp_value_get_string(arg));
+ });
+ if (!succ)
+ mgp_result_set_error_msg(
+ res, "Failed to reload the module; it is no longer loaded.");
+ };
+ mgp_proc reload("reload", reload_cb, utils::NewDeleteResource());
+ mgp_proc_add_arg(&reload, "module_name", mgp_type_string());
+ module->procedures.emplace("reload", std::move(reload));
+}
+
void RegisterMgProcedures(
// We expect modules to be sorted by name.
const std::map<std::string, Module, std::less<>> *all_modules,
@@ -140,6 +196,7 @@ void RegisterMgProcedures(
ModuleRegistry::ModuleRegistry() {
Module module{.handle = nullptr};
RegisterMgProcedures(&modules_, &module);
+ RegisterMgReload(this, &lock_, &module);
modules_.emplace("mg", std::move(module));
}