Skip to content

Commit

Permalink
fix: dec_ref being called without GIL due to singletons of py::class_
Browse files Browse the repository at this point in the history
  • Loading branch information
wu-vincent committed Mar 7, 2024
1 parent 96f657a commit 733f09a
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 55 deletions.
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ endif ()


add_compile_definitions(PYBIND11_DETAILED_ERROR_MESSAGES)
add_compile_definitions(PYBIND11_NO_ASSERT_GIL_HELD_INCREF_DECREF)


# ========
Expand Down
28 changes: 0 additions & 28 deletions include/endstone/detail/python.h

This file was deleted.

9 changes: 3 additions & 6 deletions src/endstone_python/command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include "endstone/command/command_executor.h"
#include "endstone/command/command_sender.h"
#include "endstone/detail/python.h"
#include "endstone/logger.h"
#include "endstone/server.h"

Expand Down Expand Up @@ -50,9 +49,7 @@ Command createCommand(std::string name, const std::optional<std::string> &descri

void init_command(py::module &m)
{
py_class<Server>(m, "Server");

py_class<CommandSender>(m, "CommandSender")
py::class_<CommandSender>(m, "CommandSender")
.def(
"send_message",
[](const CommandSender &sender, const std::string &message) { sender.sendMessage(message); },
Expand All @@ -61,7 +58,7 @@ void init_command(py::module &m)
"Returns the server instance that this command is running on")
.def_property_readonly("name", &CommandSender::getName, "Gets the name of this command sender");

py_class<Command, std::shared_ptr<Command>>(m, "Command")
py::class_<Command, std::shared_ptr<Command>>(m, "Command")
.def(py::init(&createCommand), py::arg("name"), py::arg("description") = py::none(),
py::arg("usages") = py::none(), py::arg("aliases") = py::none())
.def("execute", &Command::execute, py::arg("sender"), py::arg("args"),
Expand All @@ -80,7 +77,7 @@ void init_command(py::module &m)
.def_property_readonly("registered", &Command::isRegistered,
"Returns the current registered state of this command");

py_class<CommandExecutor, PyCommandExecutor, std::shared_ptr<CommandExecutor>>(m, "CommandExecutor")
py::class_<CommandExecutor, PyCommandExecutor, std::shared_ptr<CommandExecutor>>(m, "CommandExecutor")
.def(py::init<>())
.def("on_command", &CommandExecutor::onCommand, py::arg("sender"), py::arg("command"), py::arg("args"),
"Executes the given command, returning its success.");
Expand Down
2 changes: 2 additions & 0 deletions src/endstone_python/endstone_python.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include <pybind11/pybind11.h>

#include "endstone/server.h"

namespace py = pybind11;

namespace endstone::detail {
Expand Down
7 changes: 2 additions & 5 deletions src/endstone_python/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@

#include <pybind11/pybind11.h>

#include "endstone/detail/python.h"

namespace py = pybind11;

namespace endstone::detail {

void init_logger(py::module &m)
{
auto logger = py_class<Logger>(m, "Logger");
auto logger = py::class_<Logger>(m, "Logger");

py::enum_<Logger::Level>(logger, "Level")
.value("TRACE", Logger::Level::Trace)
Expand All @@ -35,8 +33,7 @@ void init_logger(py::module &m)
.value("CRITICAL", Logger::Level::Critical)
.export_values();

py_class<Logger>(m, "Logger")
.def("set_level", &Logger::setLevel, py::arg("level"), "Set the logging level for this Logger instance.")
logger.def("set_level", &Logger::setLevel, py::arg("level"), "Set the logging level for this Logger instance.")
.def("is_enabled_for", &Logger::isEnabledFor, py::arg("level"),
"Check if the Logger instance is enabled for the given log Level.")
.def(
Expand Down
14 changes: 5 additions & 9 deletions src/endstone_python/plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "endstone/detail/python.h"
#include "endstone/logger.h"
#include "endstone/plugin/plugin_loader.h"
#include "endstone/plugin/plugin_manager.h"
Expand Down Expand Up @@ -127,7 +126,7 @@ class PyPluginCommand : public PluginCommand {

void init_plugin(py::module &m)
{
py_class<PluginDescription>(m, "PluginDescription")
py::class_<PluginDescription>(m, "PluginDescription")
.def(py::init(&createPluginDescription), py::arg("name"), py::arg("version"),
py::arg("description") = py::none(), py::arg("authors") = py::none(), py::arg("prefix") = py::none())
.def_property_readonly("name", &PluginDescription::getName,
Expand All @@ -141,10 +140,7 @@ void init_plugin(py::module &m)
.def_property_readonly("prefix", &PluginDescription::getPrefix,
"Gives the token to prefix plugin-specific logging messages with.");

py_class<PluginLoader, PyPluginLoader>(m, "PluginLoader");
py_class<PluginCommand, Command, PyPluginCommand, std::shared_ptr<PluginCommand>>(m, "PluginCommand");

py_class<Plugin, CommandExecutor, PyPlugin, std::shared_ptr<Plugin>>(m, "Plugin")
py::class_<Plugin, CommandExecutor, PyPlugin, std::shared_ptr<Plugin>>(m, "Plugin")
.def(py::init<>())
.def("on_load", &Plugin::onLoad, "Called after a plugin is loaded but before it has been enabled.")
.def("on_enable", &Plugin::onEnable, "Called when this plugin is enabled")
Expand All @@ -162,20 +158,20 @@ void init_plugin(py::module &m)
.def("get_command", &Plugin::getCommand, py::return_value_policy::reference, py::arg("name"),
"Gets the command with the given name, specific to this plugin.");

py_class<PluginCommand, Command, PyPluginCommand, std::shared_ptr<PluginCommand>>(m, "PluginCommand")
py::class_<PluginCommand, Command, PyPluginCommand, std::shared_ptr<PluginCommand>>(m, "PluginCommand")
.def(py::init<const Command &, Plugin &>(), py::arg("command"), py::arg("owner"))
.def("_get_executor", &PluginCommand::getExecutor, py::return_value_policy::reference)
.def("_set_executor", &PluginCommand::setExecutor, py::arg("executor"))
.def_property_readonly("plugin", &PluginCommand::getPlugin, "Gets the owner of this PluginCommand");

py_class<PluginLoader, PyPluginLoader>(m, "PluginLoader")
py::class_<PluginLoader, PyPluginLoader>(m, "PluginLoader")
.def(py::init<Server &>(), py::arg("server"))
.def("load_plugins", &PluginLoader::loadPlugins, py::arg("directory"),
py::return_value_policy::reference_internal, "Loads the plugin contained within the specified directory")
.def("enable_plugin", &PluginLoader::enablePlugin, py::arg("plugin"), "Enables the specified plugin")
.def("disable_plugin", &PluginLoader::enablePlugin, py::arg("plugin"), "Disables the specified plugin");

py_class<PluginManager>(m, "PluginManager")
py::class_<PluginManager>(m, "PluginManager")
.def("get_plugin", &PluginManager::getPlugin, py::arg("name"), py::return_value_policy::reference,
"Checks if the given plugin is loaded and returns it when applicable.")
.def_property_readonly("plugins", &PluginManager::getPlugins, "Gets a list of all currently loaded plugins")
Expand Down
5 changes: 1 addition & 4 deletions src/endstone_python/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "endstone/detail/python.h"
#include "endstone/logger.h"
#include "endstone/plugin/plugin_manager.h"

Expand All @@ -27,9 +26,7 @@ namespace endstone::detail {

void init_server(py::module &m)
{
py_class<PluginManager>(m, "PluginManager");

py_class<Server>(m, "Server")
py::class_<Server>(m, "Server")
.def_property_readonly("logger", &Server::getLogger, py::return_value_policy::reference,
"Returns the primary logger associated with this server instance.")
.def_property_readonly("plugin_manager", &Server::getPluginManager, py::return_value_policy::reference,
Expand Down
3 changes: 1 addition & 2 deletions src/endstone_python/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

#include <pybind11/pybind11.h>

#include "endstone/detail/python.h"
#include "endstone/util/color_format.h"

namespace py = pybind11;
Expand All @@ -25,7 +24,7 @@ namespace endstone::detail {

void init_util(py::module &m)
{
py_class<ColorFormat>(m, "ColorFormat")
py::class_<ColorFormat>(m, "ColorFormat")
.ADD_COLOR_FORMAT(BLACK)
.ADD_COLOR_FORMAT(DARK_BLUE)
.ADD_COLOR_FORMAT(DARK_GREEN)
Expand Down

0 comments on commit 733f09a

Please sign in to comment.