tpaviot / pythonocc-core

Python package for 3D geometry CAD/BIM/CAM
GNU Lesser General Public License v3.0
1.39k stars 379 forks source link

BRepAdaptor_Curve causes core dump #1218

Closed hongyuan1306 closed 6 months ago

hongyuan1306 commented 1 year ago

Under pythonocc-core 7.7.0, executing Curve() on a BRepAdaptor_Curve causes a core dump:

from OCC.Core.BRepAdaptor import BRepAdaptor_Curve
from OCC.Core.BRepBuilderAPI import BRepBuilderAPI_MakeEdge
from OCC.Core.gp import gp_Pnt

def test():
    p1 = gp_Pnt(5, -5, 0)
    p2 = gp_Pnt(5, 5, 0)
    ed1 = BRepBuilderAPI_MakeEdge(p2, p1).Edge()
    c = BRepAdaptor_Curve(ed1).Curve().Curve()
    print(c)  # this line is never reached

test()
(libocct) hong@hong-office:~/workspace/libocct$ python test.py 
free(): invalid pointer
Aborted (core dumped)

(libocct) hong@hong-office:~/workspace/libocct$ conda list occ
# packages in environment at /home/hong/miniconda3/envs/libocct:
#
# Name                    Version                   Build  Channel
libocct                   1.0.0                    pypi_0    pypi
occt                      7.7.0                h6cf717c_2    conda-forge
pythonocc-core            7.7.0           py310he34791d_1    conda-forge
hongyuan1306 commented 1 year ago

This bug is the same as reported in #1176 and is likely related to #1057.

One can use BRep_Tool.Curve(edge) to get a Geom_Curve from an edge as a workaround.

SamuraiAnderson commented 1 year ago

In summary, executing BRepAdaptor_Curve(ed1).Curve() leads to a -1073740940 error on Windows. This exit code translates to 0xC0000374 in hexadecimal, a common Windows error code indicating Heap Corruption (STATUS_HEAP_CORRUPTION). Typically, this suggests that your program has written to memory that it previously allocated from the heap but has since freed.

Upon running the test function step-by-step, the print(c) command executes successfully, but it appears to be more of a lucky outcome rather than a consistent result. The -1073740940 error still occurs:

from OCC.Core.BRepAdaptor import BRepAdaptor_Curve
from OCC.Core.BRepBuilderAPI import BRepBuilderAPI_MakeEdge
from OCC.Core.gp import gp_Pnt

def test():
  p1 = gp_Pnt(5, -5, 0)
  p2 = gp_Pnt(5, 5, 0)
  ed1 = BRepBuilderAPI_MakeEdge(p2, p1).Edge()
  brepAdaptor = BRepAdaptor_Curve(ed1)
  geom_adopter = BRepAdaptor_Curve(ed1).Curve()
  c = geom_adopter.Curve()
  print(c) # this line executes

test()

It's currently unclear whether the error originates from the PythonOCC or the underlying OCCT source code. To determine the root cause, additional testing and debugging will be required.

tpaviot commented 1 year ago

In c++, the following example compiles/executes without any core dump:

#include <iostream>
#include <gp_Pnt.hxx>
#include <BRepBuilderAPI_MakeEdge.hxx>
#include <TopoDS_Edge.hxx>
#include <BRepAdaptor_Curve.hxx>
#include <Geom_Curve.hxx>

/* to compile :
$ g++ -I /opt/occt772/include/opencascade/ -L /opt/occt772/lib test_1218_cpp.cpp -o essai_1218 -lTKBRep -lTKG3d -lTKernel -lTKMath _lTKTopAlgo
*/
int main () {
    gp_Pnt p1(5, -5, 0);
    gp_Pnt p2(5, 5, 0);
    TopoDS_Edge ed1 = BRepBuilderAPI_MakeEdge(p1, p2).Edge();
    GeomAdaptor_Curve ad = BRepAdaptor_Curve(ed1).Curve();
    const opencascade::handle<Geom_Curve> h1 = ad.Curve();

    const opencascade::handle<Geom_Curve> h2 = BRepAdaptor_Curve(ed1).Curve().Curve();

    return 0;
}

In the python line

c = BRepAdaptor_Curve(ed1).Curve().Curve()

Curve() returns reference to an handle, it is possible that the python destructor forces the reference to be deleted thus being lost when calling the next Curve() call.

@rainman110

rainman110 commented 1 year ago

I think it has to do with the lifetime of objects in a call chain. I suppose, the temporary object created in the first call of curve is deleted before finishing the call to the second call.

A workaround is to not chain the calls but store the temporary curve in between.

I currently have no time to dig into it deeper, but obviously a proper Bugfix would be good

tpaviot commented 1 year ago

I agree with your diagnosis. Storing the curve in a temporary variable is actually what is suggested above:

brepAdaptor = BRepAdaptor_Curve(ed1)
geom_adopter = BRepAdaptor_Curve(ed1).Curve()
c = geom_adopter.Curve()

although there is still an issue at the end of the script, that comes IMO from the order of object deletion (the last one should be deleted first? or last?). I guess this is related to a reference/pointer somewhere to an object, I'll try to have a look but c++ memory management is definetely not my cup of tea.

rainman110 commented 1 year ago

I'll have some time next week to look into it. Maybe I can figure out, what actually happens.

tpaviot commented 1 year ago

Great!

tpaviot commented 6 months ago

Fixed by PR #1334

Unittest at a7a1d79a9348b823421c1300d732c89fe96cdcce