Uncontrolled recursion in bsp.cpp may cause stack overflow #320
Comments
|
Yes, this is a known issue. |
|
I did a proof-of-concept refactoring of the |
|
I think @Evil-Spirit had a patch for this already too, I'll need to look around in the old repository. That said, thanks anyway, this might come in handy! |
|
@whitequark, I have just optimized size of struct. I think @ghoss's patch can solve this problem in better way |
|
I combined the @ghoss changes with current master in my "issue320" branch. It compiles but crashes when I force things to triangle mesh - with models that normally work fine. This is hard to debug, as I don't know the original code, nor did I make the changes. Here is the branch: https://github.com/phkahler/solvespace/tree/issue320 and the commit: phkahler@09c71f2 |
|
The attached file can be used to replicate the problem with sphere differences. You just need to switch either of the Lathe groups to triangle mesh and then lower the chord tolerance to something like 0.01 - it will use a bunch of memory and crash. |
|
Just had this issue with 3.0~61cc28f8 with and without OpenMP on Windows 10 when a NURBS helix difference failed and needed "force to mesh". Uses about 1.5 GB RAM at this point and as NURBS, the slvs file with 500 kB becomes close to 10 MB as mesh. |
SolveSpace version: 3.0-a49a7384
Operating system: Mac OS X 10.9.5
Models with complex triangle meshes may fail to load, and/or fail to export to STL due to a stack overflow caused by recursive mutual calls of InsertConvex-InsertConvexHow, InsertHow-InsertOrCreate-Insert, InsertOrCreateEdge-InsertEdge and others in
bsp.cpp. The level of recursion is not monitored or restricted in the code, leading to the possibility of a stack overflow. In addition, memory allocation by Alloc/AllocTemporary in bsp.cpp is not controlled either, as there is no exception handling for out-of-memory conditions, nor are there tests for null pointers returned by Alloc prior to pointer assignments. Both scenarios will lead to sudden application crashes when attempting to load or export a drawing.An easy way to reproduce this is to draw two partially intersecting spheres (each created with a 180° arc and a lathe operation), and make the second sphere a "difference" of the first. This boolean operation will fail (why, would be the topic for another issue), but will be generated correctly if NURBS are forced to a triangle mesh – provided that the display chord tolerance is not too small. However, if the export chord tolerance is small enough and an export to STL is attempted, SolveSpace will crash.
Expected behavior
I would suggest implementing exception handling and/or tests for failed memory allocation in bsp.cpp. As for the recursive stack overflow issue, it might be advisable to refactor the recursive mutual functions calls as iterative loops with a local parameter stack instead of recursion, as this could be more easily monitored.
The text was updated successfully, but these errors were encountered: