add stack overflow guard in Context::eval_from #623
Conversation
Kelerchian
force-pushed
the
ada/608-stackoverflow-temp-mitigation
branch
2 times, most recently
from
883eb48
to
83102ed
Compare
| // databank process. | ||
| if let Some(x) = stacker::remaining_stack() { | ||
| if x < EVAL_REMAINING_STACK_THRESHOLD { | ||
| return Err(anyhow!( |
There was a problem hiding this comment.
I’d use anyhow::bail! in such cases.
There was a problem hiding this comment.
is there any difference between them other than one is a macro-shortcut for the other?
There was a problem hiding this comment.
no difference, it is just shorter, so easier to read
| // https://github.com/Actyx/Actyx/issues/608 | ||
| // | ||
| // see `eval_from` below | ||
| const EVAL_REMAINING_STACK_THRESHOLD: usize = 200000; |
There was a problem hiding this comment.
This number is huge, but this is because we’re only using it on one particular function call. I’m not saying this needs to be done now, but eventually we should have a remaining stack depth check at the entry of every function call controlled by user input that may be called recursively.
There was a problem hiding this comment.
It is indeed huge.
Each recursion of eval_from takes 4800-ish.
(and in this tag case, the stack overflow happens on the very first recursive call (among the 1 other in the same branch and 2 others at a different branch)
But I notice that println! stops at 170_000, there might be something going on or something unaccounted for.
There was a problem hiding this comment.
My guess is that it takes ca. 170kB stack space to run one invocation (with all its contained method calls, some of which do not call eval_from recursively). If you place such a check into all methods possibly invoked (also indirectly) by eval_from, you’ll find where the stack space is actually used.
Kelerchian
force-pushed
the
ada/608-stackoverflow-temp-mitigation
branch
2 times, most recently
from
6982512
to
4ee7c7b
Compare
Kelerchian
force-pushed
the
ada/608-stackoverflow-temp-mitigation
branch
from
4ee7c7b
to
11542be
Compare
No description provided.