-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
@@ -389,537 +390,568 @@ pub fn interpret(context: &mut Context, mut subject: Noun, formula: Noun) -> Res | |||
// | |||
// (See https://docs.rs/assert_no_alloc/latest/assert_no_alloc/#advanced-use) | |||
let nock = assert_no_alloc(|| unsafe { | |||
push_formula(&mut context.stack, formula, true)?; | |||
hw_exception::catch(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it safe to nest these invocations? What happens if a SIGSEGV results inside +mink or a parser jet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I believe nesting invocations is safe. It doesn't explicitly say so in the docs, but my understanding is that catch
closures form a stack of setjmp
buffers, and throw
will longjmp
to the one on the top of the stack. I've tested this experimentally and my results line up with this interpretation.
A SIGSEGV
in +mink
or one of the parser jets will be caught by the top-most catch
closure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "topmost" you mean "most inner" so the most recently-called +mink
would be the one to receive the error, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes; "top-most catch" ==
"inner-most +mink
"
unwrap_or is eagerly evaluated; we want unwrap_or_else which is lazily evaluated.
Note that we now have two different crates registering signal hooks. Both |
@eamsden |
@eamsden On the back of the previous comment, I'm going to play around with using |
@ashelkovnykov the point of #152 is not to catch SIGINT with |
@ashelkovnykov is this PR ready to merge? Or are you planning to also address #152 in the same PR? |
@eamsden I want to quickly see if I can improve the PR a little and unify how we handle interrupts + OOM |
@ashelkovnykov as discussed can you mark this ready-for-review and work on #152 in a separate PR? |
The nested But it also doesn't fully resolve the issue: the To work around this, @eamsden is hacking together a Frankenstein's monster crate from both |
Thinking further this isn't necessary. If we make hw_exception not allocate, and nest |
looking even further: hw_exception is overengineered for purpose and unavoidably allocates when |
I've come to the conclusion that we need a custom crate that provides unified functionality of both
So we really need these two implementations to be aware of each other. @ashelkovnykov I think work on this either ought to be suspended for now, or directed toward a |
@eamsden That sounds right - and luckily there's plenty of prior art |
Archived this PR branch here; going to submit a new PR that just adds motes. Guard page work will now be done in collaboration w/ @matthew-levan in #202. |
Resolves #172
NockStack
allocationsCurrent issues:
interpret
which were made as a result of the way in which thehw_exception::catch
closure can use local arguments (it can't use any&mut
due to extendingUnwindSafe
)