New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use of finally stack in recursive calls #2388
Comments
Case of |
Case of |
I tested maxflow and graphlets empirically earlier today, and wasn't able to get either to crash. Graphlets are probably fine too. Thanks for looking into this! |
I now reduced the finally stack use of this function, but the problem can still occur in principle. I won't spend more time on this, as the fix would take a lot of effort, and this concept of "graphlets" hasn't really caught on. The term "graphlets" is now more commonly used for an entirely unrelated idea. |
We have now encountered two instances of the following type of bug:
A recursive function uses the finally stack, causing it to overflow when the recursion depth reaches a certain level:
The symptom is the fatal error "Finally stack too large: it contains 100 elements.", which can be confusing, as it seems to suggest a mismatch between
FINALLY
andFINALLY_CLEAN
calls, but the issue is in fact an overuse of the stack.These types of bugs are typically not caught by the test suite, as triggering them requires a large graph, sometimes a large graph with a specific structure. Otherwise the recursion won't be deep enough.
Can we try to pre-emptively identify such issues? Yes,
clang-tidy
can identify recursion:You may want to not analyse C++ files, as they typically come from borrowed code that doesn't use the finally stack.
With this method, I identified the following potential problem cases. We need to check each of these for whether they are susceptible to 'finally' stack overflow, and fix them, or add a comment about why there's no issue.
local_scan_k_ecount
andlocal_scan_1_ecount
call each other, but this turns out not to be a true recursion.igraph_maximum_bipartite_matching
andigraph_i_maximum_bipartite_matching_weighted
call each other, but this turns out not to be a true recursion (see comment below).igraph_maxflow
andigraph_i_maxflow_undirected
call each other, but this turns out not to be a true recursion (see comment below).igraph_i_provan_shier_list_recursive
, fixed in igraph_cohesive_blocks() fills finally stack #2261.igraph_i_maximal_independent_vertex_sets_backtrack
, fixed in Independent vertex set finder exhausts finally stack due to recursion #2386 .igraph_i_graphlets
calls itself.Notes:
The text was updated successfully, but these errors were encountered: