Add SIGINT handling to Ares #105
Conversation
ashelkovnykov
requested review from
eamsden,
joemfb,
philipcmonk and
drbeefsupreme
| assert_no_alloc = "1.1.2" | ||
| # use this when debugging requires allocation (e.g. eprintln) | ||
| # assert_no_alloc = {version="1.1.2", features=["warn_debug"]} | ||
| bitvec = "1.0.0" | ||
| criterion = "0.4" | ||
| either = "1.9.0" | ||
| ibig = { path = "../ibig-rs" } | ||
| intmap = "1.1.0" | ||
| lazy_static = "1.4.0" | ||
| libc = "0.2.126" | ||
| murmur3 = { git = "https://github.com/tloncorp/murmur3", rev = "7878a0f" } | ||
| memmap = "0.7.0" | ||
| intmap = "1.1.0" | ||
| num-traits = "0.2" | ||
| murmur3 = { git = "https://github.com/tloncorp/murmur3", rev = "7878a0f" } | ||
| num-derive = "0.3" | ||
| criterion = "0.4" | ||
| num-traits = "0.2" | ||
| signal-hook = "0.3" |
There was a problem hiding this comment.
was this reordering done by some automated tool? Or just manually alphabetized?
There was a problem hiding this comment.
This was manual (because of the 'tism).
| @@ -480,7 +480,7 @@ impl<T: Copy + Preserve> Preserve for Hamt<T> { | |||
| }; | |||
| *(stem.buffer.add(idx) as *mut Entry<T>) = Entry { stem: new_stem }; | |||
| assert!(traversal_depth <= 5); // will increment | |||
| traversal_stack[traversal_depth - 1].unwrap().1 = position + 1; | |||
| traversal_stack[traversal_depth - 1] = Some((stem, position + 1)); | |||
There was a problem hiding this comment.
Marker for synchronous review: question about this change.
There was a problem hiding this comment.
This is fixing a logical error that was not causing a bug due to safety logic up above (we discovered it together during the very first pairing session for the memory seniority bug):
The lvalue here is incorrect and not actually resulting in the Stem in the path at depth traversal_depth - 1 to be updated. The compiler is moving the value and inferring mutability to the moved value, which is then updated, leaving the original untouched. However, when we come back around with the same position value as before, the buffer of the Stem has already been copied into the parent frame, resulting in the is_in_frame() check to return false, and therefore immediately incrementing the position value and continuing the loop.
| @@ -22,12 +23,15 @@ pub fn jet_mink( | |||
| let v_formula = slot(arg, 5)?; | |||
| let _scry = slot(arg, 3)?; | |||
|
|
|||
| // XX: NonDeterministic errors need to bail here, too | |||
| // XX: no partial traces; all of our traces go down to the "home road" | |||
There was a problem hiding this comment.
So we are switching from "Deterministic and non-deterministic traces both trace only to the nearest virtualization layer" to "Deterministic and non-deterministic traces both trace all the way to the outermost frame"?
There was a problem hiding this comment.
No, this is just documenting that all traces do not stop at virtualization layers. This was the case before this PR as well, it's just especially relevant now that interrupts are available.
| fn init() { | ||
| // This needs to be done because TERMINATOR is lazy allocated, and if you don't | ||
| // do it before you call the unit tests it'll get allocated on the Rust heap | ||
| // inside an assert_no_alloc block. | ||
| // | ||
| // Also Rust has no primitive for pre-test setup / post-test teardown, so we | ||
| // do it in a test that we rely on being called before any other in this file, | ||
| // since we're already using single-threaded test mode to avoid race conditions | ||
| // (because Rust doesn't support test order dependencies either). | ||
| let _ = Arc::clone(&TERMINATOR); |
rust/ares/src/mem.rs
Outdated
| if self.frame_pointer.is_null() | ||
| || self.stack_pointer.is_null() | ||
| || self.alloc_pointer.is_null() | ||
| { | ||
| panic!( | ||
| "serf: frame_pop: null NockStack pointer f={:p} s={:p} a={:p}", | ||
| self.frame_pointer, self.stack_pointer, self.alloc_pointer | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
panic!s can be wrapped in permit_alloc() since we know they will unwind the rust stack.
Accepting typo feedback from Ed Co-authored-by: Edward Amsden <[email protected]>
SIGINThandling:SIGINTfromking, set flag value2frame push, Nock9frame push, and mean stack pushNonDeterministicerrorSIGINTwhile the flag is set kills the process immediatelyNonDeterministicerrors to the most senior frametoddler.hoonfor easier interrupt testingNouns at Cargo level