[crash] in making 2D offset of a circle

[donovaly]

Forums discussion

https://forum.freecadweb.org/viewtopic.php?p=577963#p577963

Version

0.20 (Development)

Full version info

OS: Windows 10 Version 2009
Word size of FreeCAD: 64-bit
Version: 0.20.27990 (Git)
Build type: Release
Branch: master
Hash: c596c2fe3de933a00ba4245931e8e7bc3093832d
Python 3.8.6+, Qt 5.15.2, Coin 4.0.1, OCC 7.5.3
Locale: German/Germany (de_DE)
Installed mods:

Subproject(s) affected?

Part

Issue description

• take this simple file that contains a circle in a sketch: https://forum.freecadweb.org/download/file.php?id=182301
• create a 2D offset using the part WB and use "-0.1 mm" as offset
  result: as soon as you entered "-0.1" and then click out of the line edit of the task dialog FC crashes with illegal storage access in "BRepOffsetAPI_MakeOffset"

[xxxajk]

Also happens in the Linux version.
See FreeCAD/FreeCAD-Bundle#95 for full report.

[xxxajk]

Even though I'm not familiar with the code base at all, I at least know where the crash occurs for me, but not the why.
The crash certainly occurs at https://github.com/FreeCAD/FreeCAD/blob/master/src/Mod/Part/App/TopoShape.cpp#L2955
Is there anything I can do to help resolve this, such as instrumentation, gdb, etc?

[luzpaz]

@xxxajk please participate on the designated forum thread. Thx!

[xxxajk]

I did, left some GDB data there. This has to be an OCCT bug.

[xxxajk]

This is a major show stopper in many cases. I'm kind of surprised that this wouldn't get more rapid care, or at least identify if it is indeed an OCCT bug. If that's the case, does it also affect arcs? I have not had the chance to test that case.

[donovaly]

The problem is that we cannot do much if it is an OCC bug. We reported many bugs to the OCC guys, even patches but they don't care much. I mean there are for example patches around for more than one year and there were not considered for OCC 7.6.

[xxxajk]

I understand that, but knowing what the issue is can render a work-around.
Problem is that I don't understand the problem myself, else I would have offered a (potential) work-around.

[0penBrain]

I had a look at this. It segfaults deep in OCC and there is probably nothing we can do.
FC already implements a signal catcher but it's not enough. It catches the first segfault but stack or another part of memory stays rubbish and as soon as the function is called a second time it definitely crashes.
@donovaly Does Microsoft SEH succeed handling this one smoothly ?

[xxxajk]

For now I have resorted to doing some python code to do what I need specificlly, however it's not for normal/regular use.

That said...

If it is deep inside OOC, there's a total possibility to do the size changes in python, and generate a new or replacement toposhape. It might be a bit of work to re-implement, but is totally possible.

[donovaly]

Does Microsoft SEH succeed handling this one smoothly ?

What do you mean?
I compile with SEH set on but get the crash when creating a negative offset for a circle sketch.

[0penBrain]

@donovaly that was my question. If a Windows binary compiled with SEH crashes, there are very few chances that we can recover from the segfault. So probably useless spending time trying to catch the fault in FreeCAD. ;)

[xxxajk]

...and why I suggest writing it in Python ;-)

[donovaly]

...and why I suggest writing it in Python ;-)

The user can already draw a new circle. So there is a workaround.

However, nothing we can do not -> removing it from the milestone

[0penBrain]

@donovaly one thing we can probably do as we catch the first segfault is warn the user and advise to cancel in-progress operation, save open documents and restart FC. Is it worth?

[donovaly]

Is it worth?

If we can avoid a crash, then yes. But I cannot judge before seeing it in action.

[0penBrain]

If we can avoid a crash, then yes. But I cannot judge before seeing it in action.

In this case crash appears only on the 2nd call of the function, so yes, if user stops after 1st call, probably we can avoid the crash.
This said, AFAIK this only concerns Linux OS. Maybe we should think to a wider solution.

[donovaly]

so yes, if user stops after 1st call, probably we can avoid the crash.

This would be great. Even when it "only" cures the issue for Linux, a crash prevention is welcome.

[0penBrain]

This would be great. Even when it "only" cures the issue for Linux, a crash prevention is welcome.

I did a PoC and admittedly, this is a complete failure. :)

[xxxajk]

If we can avoid a crash, then yes. But I cannot judge before seeing it in action.

In this case crash appears only on the 2nd call of the function, so yes, if user stops after 1st call, probably we can avoid the crash. This said, AFAIK this only concerns Linux OS. Maybe we should think to a wider solution.

That's because the first call applies the changes to the exterior wires of the toposhape. The second call does any internal holes.
I'll try later to see if it affects a circle as the external wire.
Something else that I have not tested is an arc of a circle...

[FEA-eng]

The problem is that we cannot do much if it is an OCC bug. We reported many bugs to the OCC guys, even patches but they don't care much. I mean there are for example patches around for more than one year and there were not considered for OCC 7.6.

Still, maybe we could report this OCC bug. There's always a chance that they will fix this one.

[luzpaz]

Please x-post the OCC bug to this thread so we can discuss in more depth. Thanks!

[chennes]

The problem is that we cannot do much if it is an OCC bug. We reported many bugs to the OCC guys, even patches but they don't care much. I mean there are for example patches around for more than one year and there were not considered for OCC 7.6.

Still, maybe we could report this OCC bug. There's always a chance that they will fix this one.

Yes, it's worth doing, especially if we can develop a DRAW.exe test case -- and in fact, including a patch is counterproductive, since they literally cannot use it (the submitter usually has not signed the CLA, and without the copyright assignment they simply can't use the supplied code). If we have a proof-of-concept patch I can manually re-write it (so the copyright is mine) and submit it as a PR to them, though, since I have signed the CLA.

[chennes]

For reference, here's a backtrace of the failure on a debug build of OCCT 7.7.0dev:

TKTopAlgo.dll!opencascade::handle<MAT_Bisector>::handle<MAT_Bisector>(const opencascade::handle<MAT_Bisector> & theHandle) Line 68
	at occt\src\Standard\Standard_Handle.hxx(68)
TKTopAlgo.dll!MAT_TListNodeOfListOfBisector::GetItem() Line 41
	at occt\src\MAT\MAT_TListNode.lxx(41)
TKTopAlgo.dll!MAT_ListOfBisector::Current() Line 110
	at occt\src\MAT\MAT_TList.gxx(110)
TKTopAlgo.dll!MAT2d_Mat2d::CreateMat(MAT2d_Tool2d & atool) Line 1489
	at occt\src\MAT2d\MAT2d_Mat2d.cxx(1489)
TKTopAlgo.dll!BRepMAT2d_BisectingLocus::Compute(BRepMAT2d_Explorer & anExplo, const int IndexLine, const MAT_Side aSide, const GeomAbs_JoinType aJoinType, const bool IsOpenResult) Line 118
	at occt\src\BRepMAT2d\BRepMAT2d_BisectingLocus.cxx(118)
TKBool.dll!BRepFill_OffsetWire::Init(const TopoDS_Face & Spine, const GeomAbs_JoinType Join, const bool IsOpenResult) Line 383
	at occt\src\BRepFill\BRepFill_OffsetWire.cxx(383)
TKBool.dll!BRepFill_OffsetWire::BRepFill_OffsetWire(const TopoDS_Face & Spine, const GeomAbs_JoinType Join, const bool IsOpenResult) Line 341
	at occt\src\BRepFill\BRepFill_OffsetWire.cxx(341)
TKOffset.dll!BuildDomains(TopoDS_Face & myFace, NCollection_List<TopoDS_Shape> & WorkWires, NCollection_List<BRepFill_OffsetWire> & myAlgos, GeomAbs_JoinType myJoin, bool myIsOpenResult, bool isPositive, bool & isWasReversed) Line 274
	at occt\src\BRepOffsetAPI\BRepOffsetAPI_MakeOffset.cxx(274)
TKOffset.dll!BRepOffsetAPI_MakeOffset::Perform(const double Offset, const double Alt) Line 308
	at occt\src\BRepOffsetAPI\BRepOffsetAPI_MakeOffset.cxx(308)
Part_d.pyd!Part::BRepOffsetAPI_MakeOffsetFix::Perform(const double Offset, const double Alt) Line 98
	at FreeCAD\src\Mod\Part\App\BRepOffsetAPI_MakeOffsetFix.cpp(98)

[Roy-043]

FWIW Python workaround:

import Part

offset = -0.3
circle = Part.Circle().toShape()
wire = circle.makeOffset2D(-offset) # wire has one edge whose Orientation is "Reversed"
shrunk_circle = wire.makeOffset2D(2 * -offset)

Part.show(circle, "circle")
Part.show(shrunk_circle, "shrunk_circle")

[xxxajk]

Only thought now is to figure out how the "work around" can be implemented within the resize function, or I could preprocess the wires if a circle would end up a bug tripper.
I already do some processing anyway, but the idea I originally had just used OCC to do that sort of thing...
I don't really understand why it would fault -- totally doesn't make any sense whatsoever.

[bgbsww]

Here's a brute force fix that basically takes @Roy-043 's workaround, and makes it happen in the makeOffset2D code. I don't know if this should proceed, or we just wait for the eventual upstream fix.
https://github.com/bgbsww/FreeCAD/tree/bgbsww-patch-17

[xxxajk]

Personally, for what I needed to do 2D stuff, I just convert to/from shapely, which humorously is faster than OCC, since it can leverage all 24 CPUs here. I was able to process 10,000, complex edge polygons (some containing 1000 edges) in under 3 seconds instead of the ridiculous 3 to 4 hours that it took OCC to do the same exact thing, and that included the conversion between the formats.