Skip to content

Commit

Permalink
CPythonInvoker: code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@repojohnray
repojohnray committed Jan 10, 2023
1 parent 8fdbdb1 commit 83e983c
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 83 deletions.
41 changes: 14 additions & 27 deletions xbmc/interfaces/python/AddonPythonInvoker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// python.h should always be included first before any other includes
#include "AddonPythonInvoker.h"

#include <array>
#include <utility>

#include <Python.h>
Expand Down Expand Up @@ -84,45 +85,31 @@ PyObject* PyInit_Module_xbmcvfs(void);

using namespace PythonBindings;

typedef struct
namespace
{
const char *name;
CPythonInvoker::PythonModuleInitialization initialization;
} PythonModule;

static PythonModule PythonModules[] =
// clang-format off
const std::array<PythonModule, 6> PythonModules =
{
{ "xbmcdrm", PyInit_Module_xbmcdrm },
{ "xbmcgui", PyInit_Module_xbmcgui },
{ "xbmc", PyInit_Module_xbmc },
{ "xbmcplugin", PyInit_Module_xbmcplugin },
{ "xbmcaddon", PyInit_Module_xbmcaddon },
{ "xbmcvfs", PyInit_Module_xbmcvfs }
PythonModule{ "xbmcdrm", PyInit_Module_xbmcdrm },
PythonModule{ "xbmcgui", PyInit_Module_xbmcgui },
PythonModule{ "xbmc", PyInit_Module_xbmc },
PythonModule{ "xbmcplugin", PyInit_Module_xbmcplugin },
PythonModule{ "xbmcaddon", PyInit_Module_xbmcaddon },
PythonModule{ "xbmcvfs", PyInit_Module_xbmcvfs }
};
// clang-format on
} // namespace

CAddonPythonInvoker::CAddonPythonInvoker(ILanguageInvocationHandler *invocationHandler)
: CPythonInvoker(invocationHandler)
{
PyImport_AppendInittab("xbmcdrm", PyInit_Module_xbmcdrm);
PyImport_AppendInittab("xbmcgui", PyInit_Module_xbmcgui);
PyImport_AppendInittab("xbmc", PyInit_Module_xbmc);
PyImport_AppendInittab("xbmcplugin", PyInit_Module_xbmcplugin);
PyImport_AppendInittab("xbmcaddon", PyInit_Module_xbmcaddon);
PyImport_AppendInittab("xbmcvfs", PyInit_Module_xbmcvfs);
}

CAddonPythonInvoker::~CAddonPythonInvoker() = default;

std::map<std::string, CPythonInvoker::PythonModuleInitialization> CAddonPythonInvoker::getModules() const
void CAddonPythonInvoker::globalInitializeModules(void)
{
static std::map<std::string, PythonModuleInitialization> modules;
if (modules.empty())
{
for (const PythonModule& pythonModule : PythonModules)
modules.insert(std::make_pair(pythonModule.name, pythonModule.initialization));
}

return modules;
initializeModules(PythonModules);
}

const char* CAddonPythonInvoker::getInitializationScript() const
Expand Down
3 changes: 2 additions & 1 deletion xbmc/interfaces/python/AddonPythonInvoker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ class CAddonPythonInvoker : public CPythonInvoker
explicit CAddonPythonInvoker(ILanguageInvocationHandler *invocationHandler);
~CAddonPythonInvoker() override;

static void globalInitializeModules(void);

protected:
// overrides of CPythonInvoker
std::map<std::string, PythonModuleInitialization> getModules() const override;
const char* getInitializationScript() const override;
};
44 changes: 17 additions & 27 deletions xbmc/interfaces/python/PythonInvoker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ extern "C" FILE* fopen_utf8(const char* _Filename, const char* _Mode);
using namespace XFILE;
using namespace std::chrono_literals;

#define PythonModulesSize sizeof(PythonModules) / sizeof(PythonModule)

CCriticalSection CPythonInvoker::s_critical;

static const std::string getListOfAddonClassesAsString(
XBMCAddon::AddonClass::Ref<XBMCAddon::Python::PythonLanguageHook>& languageHook)
{
Expand Down Expand Up @@ -277,6 +273,7 @@ bool CPythonInvoker::execute(const std::string& script, std::vector<std::wstring
}

PySys_SetObject("argv", sysArgv);
Py_DECREF(sysArgv);

CLog::Log(LOGDEBUG, "CPythonInvoker({}, {}): entering source directory {}", GetId(), m_sourceFile,
scriptDir);
Expand Down Expand Up @@ -570,6 +567,9 @@ void CPythonInvoker::onExecutionDone()
"shutting down the Interpreter",
GetId(), m_sourceFile);

// PyErr_Clear() is required to prevent the debug python library to trigger an assert() at the Py_EndInterpreter() level
PyErr_Clear();

Py_EndInterpreter(m_threadState);

// If we still have objects left around, produce an error message detailing what's been left behind
Expand Down Expand Up @@ -619,10 +619,6 @@ void CPythonInvoker::onExecutionFailed()
void CPythonInvoker::onInitialization()
{
XBMC_TRACE;
{
GilSafeSingleLock lock(s_critical);
initializeModules(getModules());
}

// get a possible initialization script
const char* runscript = getInitializationScript();
Expand All @@ -641,15 +637,14 @@ void CPythonInvoker::onPythonModuleInitialization(void* moduleDict)

PyObject* moduleDictionary = (PyObject*)moduleDict;

PyObject* pyaddonid = PyUnicode_FromString(m_addon->ID().c_str());
PyDict_SetItemString(moduleDictionary, "__xbmcaddonid__", pyaddonid);
PyDict_SetItemString(moduleDictionary, "__xbmcaddonid__",
PyObjectPtr(PyUnicode_FromString(m_addon->ID().c_str())).get());

ADDON::CAddonVersion version = m_addon->GetDependencyVersion("xbmc.python");
PyObject* pyxbmcapiversion = PyUnicode_FromString(version.asString().c_str());
PyDict_SetItemString(moduleDictionary, "__xbmcapiversion__", pyxbmcapiversion);
PyDict_SetItemString(moduleDictionary, "__xbmcapiversion__",
PyObjectPtr(PyUnicode_FromString(version.asString().c_str())).get());

PyObject* pyinvokerid = PyLong_FromLong(GetId());
PyDict_SetItemString(moduleDictionary, "__xbmcinvokerid__", pyinvokerid);
PyDict_SetItemString(moduleDictionary, "__xbmcinvokerid__", PyLong_FromLong(GetId()));

CLog::Log(LOGDEBUG,
"CPythonInvoker({}, {}): instantiating addon using automatically obtained id of \"{}\" "
Expand Down Expand Up @@ -683,23 +678,12 @@ void CPythonInvoker::onError(const std::string& exceptionType /* = "" */,
}
}

void CPythonInvoker::initializeModules(
const std::map<std::string, PythonModuleInitialization>& modules)
{
for (const auto& module : modules)
{
if (!initializeModule(module.second))
CLog::Log(LOGWARNING, "CPythonInvoker({}, {}): unable to initialize python module \"{}\"",
GetId(), m_sourceFile, module.first);
}
}

bool CPythonInvoker::initializeModule(PythonModuleInitialization module)
bool CPythonInvoker::initializeModule(const char* name, PythonModuleInitialization module)
{
if (module == NULL)
return false;

return module() != nullptr;
return !PyImport_AppendInittab(name, module);
}

void CPythonInvoker::getAddonModuleDeps(const ADDON::AddonPtr& addon, std::set<std::string>& paths)
Expand All @@ -721,3 +705,9 @@ void CPythonInvoker::getAddonModuleDeps(const ADDON::AddonPtr& addon, std::set<s
}
}
}

void CPythonInvoker::PyObjectDeleter::operator()(PyObject* p) const
{
assert(Py_REFCNT(p) == 2);
Py_DECREF(p);
}
32 changes: 29 additions & 3 deletions xbmc/interfaces/python/PythonInvoker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <string>
#include <vector>

struct PythonModule;
typedef struct _object PyObject;

class CPythonInvoker : public ILanguageInvoker
Expand All @@ -42,7 +43,6 @@ class CPythonInvoker : public ILanguageInvoker
void onExecutionFailed() override;

// custom virtual methods
virtual std::map<std::string, PythonModuleInitialization> getModules() const = 0;
virtual const char* getInitializationScript() const = 0;
virtual void onInitialization();
// actually a PyObject* but don't wanna draw Python.h include into the header
Expand All @@ -58,9 +58,20 @@ class CPythonInvoker : public ILanguageInvoker
std::string m_sourceFile;
CCriticalSection m_critical;

protected:
template<size_t NUMBER>
static void initializeModules(const std::array<PythonModule, NUMBER> modules)
{
for (const auto& module : modules)
{
if (!initializeModule(module.name, module.initialization))
CLog::Log(LOGWARNING, "CPythonInvoker(): unable to initialize python module \"{}\"",
module.name);
}
}

private:
void initializeModules(const std::map<std::string, PythonModuleInitialization>& modules);
bool initializeModule(PythonModuleInitialization module);
static bool initializeModule(const char* name, PythonModuleInitialization module);
void getAddonModuleDeps(const ADDON::AddonPtr& addon, std::set<std::string>& paths);
bool execute(const std::string& script, std::vector<std::wstring>& arguments);
FILE* PyFile_AsFileWithMode(PyObject* py_file, const char* mode);
Expand All @@ -73,4 +84,19 @@ class CPythonInvoker : public ILanguageInvoker
bool m_systemExitThrown = false;

static CCriticalSection s_critical;

private:
struct PyObjectDeleter
{
void operator()(PyObject* p) const;
};

public:
typedef std::unique_ptr<PyObject, PyObjectDeleter> PyObjectPtr;
};

struct PythonModule
{
const char* name;
CPythonInvoker::PythonModuleInitialization initialization;
};
20 changes: 20 additions & 0 deletions xbmc/interfaces/python/XBPython.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#include "platform/Environment.h"
#endif

#ifdef HAS_WEB_INTERFACE
#include "network/httprequesthandler/python/HTTPPythonWsgiInvoker.h"
#endif

#include <algorithm>

// Only required for Py3 < 3.7
Expand All @@ -50,6 +54,14 @@ XBPython::~XBPython()
{
XBMC_TRACE;
CServiceBroker::GetAnnouncementManager()->RemoveAnnouncer(this);

#if PY_VERSION_HEX >= 0x03070000
if (Py_IsInitialized())
{
PyThreadState_Swap(PyInterpreterState_ThreadHead(PyInterpreterState_Main()));
Py_Finalize();
}
#endif
}

#define LOCK_AND_COPY(type, dest, src) \
Expand Down Expand Up @@ -518,6 +530,14 @@ bool XBPython::OnScriptInitialized(ILanguageInvoker* invoker)
Py_OptimizeFlag = 1;
#endif

// *::globalInitializeModules() functions call PyImport_AppendInittab(). PyImport_AppendInittab() should
// be called before Py_Initialize() as required by the Python documentation.
CAddonPythonInvoker::globalInitializeModules();

#ifdef HAS_WEB_INTERFACE
CHTTPPythonWsgiInvoker::globalInitializeModules();
#endif

Py_Initialize();

#if PY_VERSION_HEX < 0x03070000
Expand Down
38 changes: 14 additions & 24 deletions xbmc/network/httprequesthandler/python/HTTPPythonWsgiInvoker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "utils/StringUtils.h"
#include "utils/URIUtils.h"

#include <array>
#include <utility>

#include <Python.h>
Expand Down Expand Up @@ -75,26 +76,22 @@ PyObject* PyInit_Module_xbmcwsgi(void);

using namespace PythonBindings;

typedef struct
namespace
{
const char *name;
CPythonInvoker::PythonModuleInitialization initialization;
} PythonModule;

static PythonModule PythonModules[] =
// clang-format off
const std::array<PythonModule, 3> PythonModules =
{
{ "xbmc", PyInit_Module_xbmc },
{ "xbmcaddon", PyInit_Module_xbmcaddon },
{ "xbmcwsgi", PyInit_Module_xbmcwsgi }
PythonModule{ "xbmc", PyInit_Module_xbmc },
PythonModule{ "xbmcaddon", PyInit_Module_xbmcaddon },
PythonModule{ "xbmcwsgi", PyInit_Module_xbmcwsgi }
};
// clang-format on
} // namespace

CHTTPPythonWsgiInvoker::CHTTPPythonWsgiInvoker(ILanguageInvocationHandler* invocationHandler, HTTPPythonRequest* request)
: CHTTPPythonInvoker(invocationHandler, request),
m_wsgiResponse(NULL)
{
PyImport_AppendInittab("xbmc", PyInit_Module_xbmc);
PyImport_AppendInittab("xbmcaddon", PyInit_Module_xbmcaddon);
PyImport_AppendInittab("xbmcwsgi", PyInit_Module_xbmcwsgi);
}

CHTTPPythonWsgiInvoker::~CHTTPPythonWsgiInvoker()
Expand All @@ -103,6 +100,11 @@ CHTTPPythonWsgiInvoker::~CHTTPPythonWsgiInvoker()
m_wsgiResponse = NULL;
}

void CHTTPPythonWsgiInvoker::globalInitializeModules(void)
{
initializeModules(PythonModules);
}

HTTPPythonRequest* CHTTPPythonWsgiInvoker::GetRequest()
{
if (m_request == NULL || m_wsgiResponse == NULL)
Expand Down Expand Up @@ -303,18 +305,6 @@ void CHTTPPythonWsgiInvoker::executeScript(FILE* fp, const std::string& script,
}
}

std::map<std::string, CPythonInvoker::PythonModuleInitialization> CHTTPPythonWsgiInvoker::getModules() const
{
static std::map<std::string, PythonModuleInitialization> modules;
if (modules.empty())
{
for (const PythonModule& pythonModule : PythonModules)
modules.insert(std::make_pair(pythonModule.name, pythonModule.initialization));
}

return modules;
}

const char* CHTTPPythonWsgiInvoker::getInitializationScript() const
{
return RUNSCRIPT;
Expand Down
3 changes: 2 additions & 1 deletion xbmc/network/httprequesthandler/python/HTTPPythonWsgiInvoker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ class CHTTPPythonWsgiInvoker : public CHTTPPythonInvoker
CHTTPPythonWsgiInvoker(ILanguageInvocationHandler* invocationHandler, HTTPPythonRequest* request);
~CHTTPPythonWsgiInvoker() override;

static void globalInitializeModules(void);

// implementations of CHTTPPythonInvoker
HTTPPythonRequest* GetRequest() override;

protected:
// overrides of CPythonInvoker
void executeScript(FILE* fp, const std::string& script, PyObject* moduleDict) override;
std::map<std::string, PythonModuleInitialization> getModules() const override;
const char* getInitializationScript() const override;

private:
Expand Down

0 comments on commit 83e983c

Please sign in to comment.