Skip to content

Commit b4db88e

Browse files
committed
Revert "Fix Python reference counting for declared_transform product store caching (#313)"
This reverts commit e348b4d.
1 parent e348b4d commit b4db88e

3 files changed

Lines changed: 18 additions & 226 deletions

File tree

plugins/python/src/lifelinewrap.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ static py_lifeline_t* ll_new(PyTypeObject* pytype, PyObject*, PyObject*)
99
{
1010
py_lifeline_t* pyobj = (py_lifeline_t*)pytype->tp_alloc(pytype, 0);
1111
if (!pyobj)
12-
return nullptr;
12+
PyErr_Print();
1313
pyobj->m_view = nullptr;
1414
new (&pyobj->m_source) std::shared_ptr<void>{};
1515

plugins/python/src/modulewrap.cpp

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,6 @@ namespace {
107107

108108
PyGILRAII gil;
109109

110-
// Args are borrowed references from the product store cache.
111-
// XINCREF to create temporary owned references for the duration of the call.
112-
(Py_XINCREF((PyObject*)args), ...);
113-
114110
PyObject* result =
115111
PyObject_CallFunctionObjArgs(m_callable, lifeline_transform(args)..., nullptr);
116112

@@ -120,8 +116,7 @@ namespace {
120116
error_msg = "Unknown python error";
121117
}
122118

123-
// Release our temporary references; the cache's references remain intact.
124-
(Py_XDECREF((PyObject*)args), ...);
119+
decref_all(args...);
125120

126121
if (!error_msg.empty()) {
127122
throw std::runtime_error(error_msg.c_str());
@@ -137,12 +132,7 @@ namespace {
137132

138133
PyGILRAII gil;
139134

140-
// Args are borrowed references from the product store cache.
141-
// XINCREF to create temporary owned references for the duration of the call.
142-
(Py_XINCREF((PyObject*)args), ...);
143-
144-
PyObject* result =
145-
PyObject_CallFunctionObjArgs(m_callable, lifeline_transform(args)..., nullptr);
135+
PyObject* result = PyObject_CallFunctionObjArgs(m_callable, (PyObject*)args..., nullptr);
146136

147137
std::string error_msg;
148138
if (!result) {
@@ -151,13 +141,20 @@ namespace {
151141
} else
152142
Py_DECREF(result);
153143

154-
// Release our temporary references; the cache's references remain intact.
155-
(Py_XDECREF((PyObject*)args), ...);
144+
decref_all(args...);
156145

157146
if (!error_msg.empty()) {
158147
throw std::runtime_error(error_msg.c_str());
159148
}
160149
}
150+
151+
private:
152+
template <typename... Args>
153+
void decref_all(Args... args)
154+
{
155+
// helper to decrement reference counts of N arguments
156+
(Py_DECREF((PyObject*)args), ...);
157+
}
161158
};
162159

163160
// use explicit instatiations to ensure that the function signature can
@@ -336,13 +333,14 @@ namespace {
336333
\
337334
static cpptype py_to_##name(intptr_t pyobj) \
338335
{ \
339-
/* Input is a borrowed reference from the product store cache — do not DECREF. */ \
340336
PyGILRAII gil; \
341337
cpptype i = (cpptype)frompy((PyObject*)pyobj); \
342338
std::string msg; \
343339
if (msg_from_py_error(msg, true)) { \
340+
Py_DECREF((PyObject*)pyobj); \
344341
throw std::runtime_error("Python conversion error for type " #name ": " + msg); \
345342
} \
343+
Py_DECREF((PyObject*)pyobj); \
346344
return i; \
347345
}
348346

@@ -360,7 +358,7 @@ namespace {
360358
PyGILRAII gil; \
361359
\
362360
if (!v) \
363-
throw std::runtime_error("null vector<" #cpptype "> passed to " #name "_to_py"); \
361+
return (intptr_t)nullptr; \
364362
\
365363
/* use a numpy view with the shared pointer tied up in a lifeline object (note: this */ \
366364
/* is just a demonstrator; alternatives are still being considered) */ \
@@ -373,7 +371,7 @@ namespace {
373371
); \
374372
\
375373
if (!np_view) \
376-
throw std::runtime_error("failed to create numpy array in " #name "_to_py"); \
374+
return (intptr_t)nullptr; \
377375
\
378376
/* make the data read-only by not making it writable */ \
379377
PyArray_CLEARFLAGS((PyArrayObject*)np_view, NPY_ARRAY_WRITEABLE); \
@@ -385,7 +383,7 @@ namespace {
385383
(py_lifeline_t*)PhlexLifeline_Type.tp_new(&PhlexLifeline_Type, nullptr, nullptr); \
386384
if (!pyll) { \
387385
Py_DECREF(np_view); \
388-
throw std::runtime_error("failed to create lifeline in " #name "_to_py"); \
386+
return (intptr_t)nullptr; \
389387
} \
390388
pyll->m_source = v; \
391389
pyll->m_view = np_view; /* steals reference */ \
@@ -403,7 +401,6 @@ namespace {
403401
#define NUMPY_ARRAY_CONVERTER(name, cpptype, nptype, frompy) \
404402
static std::shared_ptr<std::vector<cpptype>> py_to_##name(intptr_t pyobj) \
405403
{ \
406-
/* Input is a borrowed reference from the product store cache — do not DECREF. */ \
407404
PyGILRAII gil; \
408405
\
409406
auto vec = std::make_shared<std::vector<cpptype>>(); \
@@ -441,6 +438,7 @@ namespace {
441438
} \
442439
} \
443440
\
441+
Py_DECREF((PyObject*)pyobj); \
444442
return vec; \
445443
}
446444

plugins/python/src/python-refcounting.md

Lines changed: 0 additions & 206 deletions
This file was deleted.

0 commit comments

Comments
 (0)