coin3d / pivy

python bindings to coin3d
ISC License
53 stars 37 forks source link

Tests no longer segfault on Python 3.10 #91

Closed antonok-edm closed 2 years ago

antonok-edm commented 2 years ago

Fixes https://github.com/coin3d/pivy/issues/88.

Had to do a ton of debugging and doc-hunting for this one.

According to the PyArg_ParseTuple() docs:

int PyArg_ParseTuple(PyObject *arg, const char *format, ...);

The arg argument must be a tuple object containing an argument list passed from Python to a C function.

autocast_base, autocast_path, autocast_field, autocast_event were all building a new tuple to take advantage of code reuse with the cast function, which would then re-parse the tuple using PyArg_ParseTuple. As per the docs, PyArg_ParseTuple should only ever be passed an object directly from Python. Receiving the newly built tuple was causing segfaults.

I've solved this by creating a new unexported cast_internal function that gets called by all 5 of the above functions, directly taking the objects from inside an already-parsed tuple. PyArg_ParseTuple is now only called by the SWIG-exported functions.

In addition to running the tests successfully, I've confirmed that this fixes the issues with the FreeCAD Path, Draft, and Architecture workbenches on Arch Linux.

hobbes1069 commented 2 years ago

Yay! This helped building for Fedora Rawhide with 3.10 (3.11 coming soon!)

natask commented 2 years ago

After building and installing pivy with this commit, freecad path works but draft and architecture workbenches crash freecad. I am on freecad --version FreeCAD 0.19 Revision: 24291 (Git) (0.19.3-12 community arch) and python --version Python 3.10.1.

antonok-edm commented 2 years ago

@natask could you post a traceback for the draft and architecture workbenches? They work fine for me and I suspect it's a separate issue, but it'd be good to have for the record at least

natask commented 2 years ago

@antonok-edm I get the following for draft workbench. segfault in the architecture workbench also has the same structure.

Program received signal SIGSEGV, Segmentation fault.
#0  /usr/lib/libc.so.6(+0x3cda0) [0x7fda22f08da0]
#1  /usr/lib/libc.so.6(+0x16206e) [0x7fda2302e06e]
#2  0x7fd9de310764 in cast(_object*, _object*) from /usr/lib/python3.10/site-packages/pivy/_coin.cpython-310-x86_64-linux-gnu.so+0x84
#3  0x7fd9de40329a in autocast_field(SoField*) from /usr/lib/python3.10/site-packages/pivy/_coin.cpython-310-x86_64-linux-gnu.so+0xca
#4  /usr/lib/python3.10/site-packages/pivy/_coin.cpython-310-x86_64-linux-gnu.so(+0x5daf43) [0x7fd9de403f43]
#5  /usr/lib/libpython3.10.so.1.0(+0x1450f8) [0x7fda2464f0f8]
#6  /usr/lib/libpython3.10.so.1.0(_PyObject_MakeTpCall+0x2a3) [0x7fda246496b3]
#7  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x582b) [0x7fda24644e7b]
#8  /usr/lib/libpython3.10.so.1.0(+0x150ac0) [0x7fda2465aac0]
#9  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x4f0f) [0x7fda2464455f]
#10  /usr/lib/libpython3.10.so.1.0(+0x150ac0) [0x7fda2465aac0]
#11  /usr/lib/libpython3.10.so.1.0(+0x193360) [0x7fda2469d360]
#12  /usr/lib/libpython3.10.so.1.0(+0x20933e) [0x7fda2471333e]
#13  /usr/lib/libpython3.10.so.1.0(+0x1a2648) [0x7fda246ac648]
#14  /usr/lib/libpython3.10.so.1.0(PyObject_GetAttr+0x52) [0x7fda2464c662]
#15  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x4d25) [0x7fda24644375]
#16  /usr/lib/libpython3.10.so.1.0(_PyObject_FastCallDictTstate+0xd4) [0x7fda24648964]
#17  /usr/lib/libpython3.10.so.1.0(+0x14d787) [0x7fda24657787]
#18  /usr/lib/libpython3.10.so.1.0(_PyObject_MakeTpCall+0x27b) [0x7fda2464968b]
#19  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x582b) [0x7fda24644e7b]
#20  /usr/lib/libpython3.10.so.1.0(_PyFunction_Vectorcall+0x78) [0x7fda2464f538]
#21  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x655) [0x7fda2463fca5]
#22  /usr/lib/libpython3.10.so.1.0(+0x150cc5) [0x7fda2465acc5]
#23  /usr/lib/freecad/lib/libFreeCADBase.so(PP_Run_Method+0xee) [0x7fda24a55b8e]
#24  0x7fda249dcee0 in Base::InterpreterSingleton::runMethodVoid(_object*, char const*) from /usr/lib/freecad/lib/libFreeCADBase.so+0x30
#25  0x7fda25389e0d in Gui::PythonCommand::activated(int) from /usr/lib/freecad/lib/libFreeCADGui.so+0xbd
#26  0x7fda2538a648 in Gui::Command::invoke(int, Gui::Command::TriggerSource) from /usr/lib/freecad/lib/libFreeCADGui.so+0x528
#27  0x7fda253006f3 in Gui::Application::sRunCommand(_object*, _object*) from /usr/lib/freecad/lib/libFreeCADGui.so+0x93
#28  /usr/lib/libpython3.10.so.1.0(+0x1450f8) [0x7fda2464f0f8]
#29  /usr/lib/libpython3.10.so.1.0(_PyObject_MakeTpCall+0x2a3) [0x7fda246496b3]
#30  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x582b) [0x7fda24644e7b]
#31  /usr/lib/libpython3.10.so.1.0(+0x134602) [0x7fda2463e602]
#32  /usr/lib/libpython3.10.so.1.0(PyEval_EvalCode+0x90) [0x7fda246f28a0]
#33  /usr/lib/libpython3.10.so.1.0(+0x1f9484) [0x7fda24703484]
#34  /usr/lib/libpython3.10.so.1.0(+0x1f4f5b) [0x7fda246fef5b]
#35  /usr/lib/libpython3.10.so.1.0(PyRun_StringFlags+0x7d) [0x7fda246f71bd]
#36  0x7fda2530083f in Gui::Application::sDoCommand(_object*, _object*) from /usr/lib/freecad/lib/libFreeCADGui.so+0xbf
#37  /usr/lib/libpython3.10.so.1.0(+0x1450f8) [0x7fda2464f0f8]
#38  /usr/lib/libpython3.10.so.1.0(_PyObject_MakeTpCall+0x2a3) [0x7fda246496b3]
#39  /usr/lib/libpython3.10.so.1.0(_PyEval_EvalFrameDefault+0x51c6) [0x7fda24644816]
#40  /usr/lib/libpython3.10.so.1.0(_PyFunction_Vectorcall+0x78) [0x7fda2464f538]
#41  0x7fda2247948e in PySide::SignalManager::callPythonMetaMethod(QMetaMethod const&, void**, _object*, bool) from /usr/lib/libpyside2.cpython-310-x86_64-linux-gnu.so.5.15+0x9e
#42  /usr/lib/libpyside2.cpython-310-x86_64-linux-gnu.so.5.15(+0x13dc8) [0x7fda2247adc8]
#43  /usr/lib/libQt5Core.so.5(+0x2bc394) [0x7fda23585394]
#44  0x7fda23fbb417 in QAction::triggered(bool) from /usr/lib/libQt5Widgets.so.5+0x47
#45  0x7fda23fc0eb0 in QAction::activate(QAction::ActionEvent) from /usr/lib/libQt5Widgets.so.5+0xb0
#46  /usr/lib/libQt5Widgets.so.5(+0x26357d) [0x7fda240b257d]
#47  0x7fda240b27a8 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) from /usr/lib/libQt5Widgets.so.5+0xb8
#48  0x7fda241a20bf in QToolButton::mouseReleaseEvent(QMouseEvent*) from /usr/lib/libQt5Widgets.so.5+0xf
#49  0x7fda23ffe616 in QWidget::event(QEvent*) from /usr/lib/libQt5Widgets.so.5+0x896
#50  0x7fda23fc81a6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) from /usr/lib/libQt5Widgets.so.5+0x86
#51  0x7fda23fccfd7 in QApplication::notify(QObject*, QEvent*) from /usr/lib/libQt5Widgets.so.5+0xb67
#52  0x7fda25363431 in Gui::GUIApplication::notify(QObject*, QEvent*) from /usr/lib/freecad/lib/libFreeCADGui.so+0x91
#53  0x7fda235549ba in QCoreApplication::notifyInternal2(QObject*, QEvent*) from /usr/lib/libQt5Core.so.5+0x13a
#54  0x7fda23fcb99f in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) from /usr/lib/libQt5Widgets.so.5+0x1bf
#55  /usr/lib/libQt5Widgets.so.5(+0x1cd837) [0x7fda2401c837]
#56  /usr/lib/libQt5Widgets.so.5(+0x1cf3dc) [0x7fda2401e3dc]
#57  0x7fda23fc81a6 in QApplicationPrivate::notify_helper(QObject*, QEvent*) from /usr/lib/libQt5Widgets.so.5+0x86
#58  0x7fda25363431 in Gui::GUIApplication::notify(QObject*, QEvent*) from /usr/lib/freecad/lib/libFreeCADGui.so+0x91
#59  0x7fda235549ba in QCoreApplication::notifyInternal2(QObject*, QEvent*) from /usr/lib/libQt5Core.so.5+0x13a
#60  0x7fda23925f60 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) from /usr/lib/libQt5Gui.so.5+0x570
#61  0x7fda239115e5 in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) from /usr/lib/libQt5Gui.so.5+0xb5
#62  /usr/lib/libQt5XcbQpa.so.5(+0x63d90) [0x7fda1db42d90]
#63  /usr/lib/libglib-2.0.so.0(g_main_context_dispatch+0x193) [0x7fda21344fd3]
#64  /usr/lib/libglib-2.0.so.0(+0xab049) [0x7fda2139b049]
#65  /usr/lib/libglib-2.0.so.0(g_main_context_iteration+0x35) [0x7fda21342545]
#66  0x7fda2359fc8a in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) from /usr/lib/libQt5Core.so.5+0x6a
#67  0x7fda2354cbab in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) from /usr/lib/libQt5Core.so.5+0x12b
#68  0x7fda235583e7 in QCoreApplication::exec() from /usr/lib/libQt5Core.so.5+0x97
#69  0x7fda252e01f7 in Gui::Application::runApplication() from /usr/lib/freecad/lib/libFreeCADGui.so+0x16c7
#70  freecad(+0x38f8) [0x562d4c9168f8]
#71  /usr/lib/libc.so.6(__libc_start_main+0xd5) [0x7fda22ef3b25]
#72  freecad(+0x4a0e) [0x562d4c917a0e]
antonok-edm commented 2 years ago

@natask looking at these two lines:

#2  0x7fd9de310764 in cast(_object*, _object*) from /usr/lib/python3.10/site-packages/pivy/_coin.cpython-310-x86_64-linux-gnu.so+0x84
#3  0x7fd9de40329a in autocast_field(SoField*) from /usr/lib/python3.10/site-packages/pivy/_coin.cpython-310-x86_64-linux-gnu.so+0xca

I believe your FreeCAD is still calling into the old pivy version - this PR removed the autocast_field -> cast call path. The callstack in this branch would look like autocast_field(SoField*) -> cast_internal(PyObject*, PyObject*, const char*, int).

I fumbled around with $PYTHONPATH for a while but none of that worked for me. What I ultimately did was directly download the PKGBUILD for python-pivy, make a couple of tweaks, and run makepkg -si to install it.

This should work:

diff PKGBUILD.bak PKGBUILD
4c4
< pkgname=python-pivy
---
> pkgname=python-pivy-git
10c10
< url='https://github.com/coin3d/pivy'
---
> url='https://github.com/antonok-edm/pivy'
13c13
< source=("git+$url#commit=55e659de7ea346d3caf176d7fe254224d36e4791") # tag: 0.6.6
---
> source=("git+$url#commit=377d97ce41519e879a6c0111f1193957653a06ae")
14a15,16
> provides=('python-pivy')
> conflicts=('python-pivy')
ipatch commented 2 years ago

for those of us who are still using python 3.9.x, will PR still work with python 3.9.x releaes?

antonok-edm commented 2 years ago

@ipatch I haven't tested it, but it should! I haven't added any new Python API calls here (in fact I've removed some!), just changed the order in which existing ones are called.

looooo commented 2 years ago

Thanks @antonok-edm for diving into this issue.

luzpaz commented 2 years ago

Does this fix merit a minor release?

looooo commented 2 years ago

yes I guess so. But let's wait a little bit to see if everything works now with python3.10

looooo commented 2 years ago

@antonok-edm we see an issue with this PR:

we see a crash with this minimal example:

from pivy import coin
det = coin.SoFaceDetail()
coin.cast(det, "SoFaceDetail")

maybe you can help with finding a solution for this.

ipatch commented 2 years ago

@looooo

i tried your given code snippet with the latest homebrew build and i get the below,

freecad about info ``` OS: macOS 10.15 Word size of FreeCAD: 64-bit Version: 0.20.28901 +34 (Git) Build type: Release Branch: ipatch.builds.0.20beta1 Hash: 8505cf72df8794d39f847939f751b2d0c15c57a5 Python 3.10.4, Qt 5.15.3, Coin 4.0.1, OCC 7.6.2 Locale: English/United States (en_US) Installed mods: * Behave-Dark-Colors 0.0.1 * Plot * CfdOF * Defeaturing * CfdOF.bak * A2plus 0.4.56 * Manipulator 1.4.3 * fasteners 0.3.40 * Help 1.0.0-alpha * Curves 0.4.4 ```
>>> from pivy import coin
>>> det = coin.SoFaceDetail()
>>> coin.cast(det, "SoFaceDetail")
<pivy.coin.SoFaceDetail; proxy of <Swig Object of type 'SoFaceDetail *' at 0x116198a20> >
>>> Gui.runCommand('Std_About',0)
>>> 
>>> import pivy as pvy
>>> print(pvy.__version__)
0.6.7

>>> from pivy import coin as coins
>>> print(coins.COIN_MAJOR_VERSION)
4
>>> print(coins.COIN_MINOR_VERSION)
0
>>> print(coins.COIN_MICRO_VERSION)
1
antonok-edm commented 2 years ago

we see a crash with this minimal example:

from pivy import coin
det = coin.SoFaceDetail()
coin.cast(det, "SoFaceDetail")

@looooo I'd be very surprised if the issue was introduced with this PR, given that the exact same code is called (just rearranged into two functions instead of one).

That said, I'm able to reproduce this issue too, and I tried digging into it for a bit.

If we look at the C++-side definition of cast:

/* a casting helper function */
SWIGEXPORT PyObject *
cast(PyObject * self, PyObject * args)
{
  char * type_name;
  int type_len;
  PyObject * obj = 0;

  if (!PyArg_ParseTuple(args, "Os#:cast", &obj, &type_name, &type_len)) {
    SWIG_fail;
  }

  return cast_internal(self, obj, type_name, type_len);
fail:
  return NULL;
}

PyArgParseTuple(args, "Os#:cast", &obj, &type_name, &type_len) returns with a success status, and gdb indicates that obj is populated with a value of 0x7fff00000000 (which I'm 95% sure is the PyObject equivalent of a null pointer). Replacing det with any other Python value in your minimal example above produces the exact same result for me.

I see <pivy.coin.SoFaceDetail; proxy of <Swig Object of type 'SoFaceDetail *' at 0x7f728e29b000> > when calling print(det), so it should be able to produce a valid C pointer. The O format unit (from "Os#:cast") is supposed to provide the following behavior:

Store a Python object (without any conversion) in a C object pointer. The C program thus receives the actual object that was passed. The object’s reference count is not increased. The pointer stored is not NULL.

The fact that every value I tried to pass in as the first argument continued to be parsed as a 0x7fff00000000 pointer goes against any intuition I had about the behavior of PyArgParseTuple's O format unit, and I don't see any other format units that might be more appropriate to use here, so I'm unfortunately not sure what's happening here.

marioalexis84 commented 1 year ago

See https://github.com/coin3d/pivy/pull/99