| Summary: | kig crashes with simple python script with a failing assertion | ||
|---|---|---|---|
| Product: | [Applications] kig | Reporter: | Maurizio Paolini <maurizio.paolini> |
| Component: | general | Assignee: | David E. Narvaez <david.narvaez> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | allo, franco.pasquarelli, kevin.kofler, rdieter |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Fedora RPMs | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Attachments: | kig file that makes kig crash | ||
It seems that the first appearance of strange values for Py_REFCNT appear in the retdict.get function called within CompiledPythonScript PythonScripter::compile( const char* code ) if file scripting/python_scripter.cc ---------------- snippet --------------- ret->ref = 0; ret->calcfunc = retdict.get( "calc" ); ---------------------------------------- I don't know whether the problem is really there and how to debug further The following diff:
-----------------------------------------------------
$ git diff
diff --git a/scripting/python_scripter.cc b/scripting/python_scripter.cc
index 62a7409b..57e09255 100644
--- a/scripting/python_scripter.cc
+++ b/scripting/python_scripter.cc
@@ -531,6 +531,10 @@ ObjectImp* PythonScripter::calc( CompiledPythonScript& script, const Args& args
};
tuple argstup( argstuph );
+// workaround?
+//
+ Py_INCREF (argstup.ptr());
+
handle<> reth( PyEval_CallObject( calcfunc.ptr(), argstup.ptr() ) );
// object resulto = calcfunc( argstup );
// handle<> reth( PyEval_CallObject( calcfunc.ptr(), args ) );
-----------------------------------------------------
seems to "work around" the reported issue. I do not know if this is a real fix because I have no experience with boost, but at least allows to use python scripting that otherwise seems to be completely broken (see the attached kig example)
*** Bug 409497 has been marked as a duplicate of this bug. *** So, after looking at the code and the documentation, I suspect the issue is not really the reference count of the tuple itself, but of the individual arguments, which would explain why you do not hit this assertion if the function has no arguments. Also the following diff solve the problem
diff --git a/scripting/python_scripter.cc b/scripting/python_scripter.cc
index 62a7409b..ada48861 100644
--- a/scripting/python_scripter.cc
+++ b/scripting/python_scripter.cc
@@ -527,13 +527,14 @@ ObjectImp* PythonScripter::calc( CompiledPythonScript& script, const Args& args
handle<> argstuph( PyTuple_New( args.size() ) );
for ( int i = 0; i < (int) objectvect.size(); ++i )
{
+ Py_XINCREF((objectvect.begin() +i)->ptr());
PyTuple_SetItem( argstuph.get(), i, (objectvect.begin() +i)->ptr() );
};
+
+
tuple argstup( argstuph );
handle<> reth( PyEval_CallObject( calcfunc.ptr(), argstup.ptr() ) );
-// object resulto = calcfunc( argstup );
-// handle<> reth( PyEval_CallObject( calcfunc.ptr(), args ) );
object resulto( reth );
extract<ObjectImp&> result( resulto );
where I increase the reference count of the arguments
(In reply to Kevin Kofler from comment #4) > So, after looking at the code and the documentation, I suspect the issue is > not really the reference count of the tuple itself, but of the individual > arguments, which would explain why you do not hit this assertion if the > function has no arguments. I agree that the issue seems to be the refcount of the arguments, but how would you explain that the patch in comment #2 works around the issue? On a separate note, the assertion was added 3 years ago here https://github.com/boostorg/python/commit/bc2f77a3db0b3b428ef7bc205804ce5625e46001 so it seems this has been broken since Boost 1.63 or so? The patch from comment #2 prevents the tuple from being de-refcounted all the way down to 0 (because it adds a bogus unowned reference), so the tuple is leaked, and its continued existence also keeps the arguments referenced. (It is the destruction of the tuple that decreases the reference count of each argument.) On the other hand, the patch from comment #5 looks like the correct fix to me. (In reply to Kevin Kofler from comment #7) > The patch from comment #2 prevents the tuple from being de-refcounted all > the way down to 0 (because it adds a bogus unowned reference), so the tuple > is leaked, and its continued existence also keeps the arguments referenced. > (It is the destruction of the tuple that decreases the reference count of > each argument.) Ah, makes sense. > On the other hand, the patch from comment #5 looks like the correct fix to > me. I agree. Franco, can you post your patch to Phabricator? Against the release/19.12 branch please. (In reply to David E. Narvaez from comment #6) > (In reply to Kevin Kofler from comment #4) > > So, after looking at the code and the documentation, I suspect the issue is > > not really the reference count of the tuple itself, but of the individual > > arguments, which would explain why you do not hit this assertion if the > > function has no arguments. > > I agree that the issue seems to be the refcount of the arguments, but how > would you explain that the patch in comment #2 works around the issue? > > On a separate note, the assertion was added 3 years ago here > > https://github.com/boostorg/python/commit/ > bc2f77a3db0b3b428ef7bc205804ce5625e46001 > > so it seems this has been broken since Boost 1.63 or so? Yes, indeed! I am no longer an active user, so I realized the presence of the problem after some time; then I posted the report only downstream for fedora. In any case I opened this bug report more than one year ago. (In reply to David E. Narvaez from comment #8) > (In reply to Kevin Kofler from comment #7) > > The patch from comment #2 prevents the tuple from being de-refcounted all > > the way down to 0 (because it adds a bogus unowned reference), so the tuple > > is leaked, and its continued existence also keeps the arguments referenced. > > (It is the destruction of the tuple that decreases the reference count of > > each argument.) > > Ah, makes sense. > > > On the other hand, the patch from comment #5 looks like the correct fix to > > me. > > I agree. Franco, can you post your patch to Phabricator? Against the > release/19.12 branch please. On behalf of Franco I created a task on phabricator: https://phabricator.kde.org/T12453 however I am new to phabricator, so I don't know how to proceed. actually, the patch is in https://phabricator.kde.org/differential/revision/edit/26333/ I am not sure that I am using phabricator correctly and couldn't find a place where to indicate that it should be used against release/19.12 (In reply to Maurizio Paolini from comment #11) > actually, the patch is in > [WRONG] > https://phabricator.kde.org/differential/revision/edit/26333/ > > I am not sure that I am using phabricator correctly and couldn't find a > place where to indicate that it should be used against release/19.12 oops, pardon me, the URL of the revision is: https://phabricator.kde.org/D26333 |
Created attachment 116551 [details] kig file that makes kig crash SUMMARY kig crashes with: kig: /usr/include/boost/python/object_core.hpp:422: boost::python::api::object_base::~object_base(): Assertion `Py_REFCNT(m_ptr) > 0' failed. when trying to define a simple python script with a numeric label as single argument and returning a DoubleObject or loading a kig file with such a construction. STEPS TO REPRODUCE 1. Download the attachment "refcnt_bug.kig" 2. start kig with kig refcnt_bug.kig OBSERVED RESULT crash with the message: kig: /usr/include/boost/python/object_core.hpp:422: boost::python::api::object_base::~object_base(): Assertion `Py_REFCNT(m_ptr) > 0' failed. SOFTWARE/OS VERSIONS I am using a Fedora 29 installation fully updated. KDE Framework is kf5-5.50 QT5 5.11 boost-devel-1.66.0-14.fc29.x86_64 kig was downloaded from latest Git repository and compiled from source. Python scripting support was correctly detected. ADDITIONAL INFORMATION The failing assert is contained in /usr/include/boost/python/object_core.hpp a file contained in the Fedora package boost-devel-1.66.0-14.fc29.x86_64: inline api::object_base::~object_base() { assert( Py_REFCNT(m_ptr) > 0 ); Py_DECREF(m_ptr); } It I comment the assert and recompile everything works apparently fine, however the value of Py_REFCNT becomse -1 and then gets quite strange values (seen by adding a printf in place of the assert) indicating memory allocation problems.