There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. An I/O error should fail softly.
There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. An I/O error should fail softly.
nah. in GUI apps for instance you want the failure in the logic of a sub-sub-function to just tell the error "wops" when the button that triggered the action was clicked, not nuke the app (unless you hate your users). e.g. imagine a 3D software which allows to do mesh operations - user clicks on the "Smooth the mesh" button somewhere. Programmer forgot to handle a division by zero in some degenerate case of the smoothing computation which ends up leading to an exception: a value becomes zero, someone used unsigned integers for n in an "n - 1" computation which ends up in a call to array_of_floats.resize(0xffffffffffffffff) (and a likely std::bad_alloc being thrown if you're in c++).
The original mesh is unchanged as the operation waits until the computation is complete to replace the old mesh with the new.
If you ever decide to crash in this situation I am sure you will have great reviews on 3D modeling software comparisons.
To me this is a textbook case of why panic::catch_unwind exists in Rust. Conceptually, the smoothing algorithm failed in an unexpected way, so a panic is appropriate: there is nothing else for the procedure to do but crash. But crashing would be a very poor user experience, because the smoothing operation is conceptually isolated from the application as a whole, so the program shouldn't be affected by a single operation crashing. This is why Rust goes to the trouble to implement unwinding on panic: in certain domains, software fault isolation is important.
Another possibility, of course, would be to spawn a separate process for the smoothing operation, which would effectively replace software fault isolation with hardware fault isolation. But this might be awkward (and slow), which is why few modeling software packages do this.
("A complex smoothing operation in a 3D modeling package fails" is maybe the best example of the need for panic::catch_unwind I've ever heard of--thanks for offering it. I wish I had been able to deploy this example back when we had the debate as to whether catch_unwind should exist at all. Thankfully, we kept it in.) :)
I do agree this is a much better model, and it doesn't have to be particularly slow or painful. An example system that does this well is XPC on macOS [1]. The nice thing is by defining a good service model is you can also run your third-party plugins the same way - with full process-level privilege isolation - and in the event of a crash the service can simply be restarted.
[1] https://developer.apple.com/library/archive/documentation/Ma...
Now, it may be true that you get better reviews selling software that corrupts data or gets its users hacked than selling software that crashes. But unless you hate your users, those are probably things you want to avoid.
Instead, the bigger issue is corrupt external state, e.g. in a database. But that can of course happen just as well if you panic. What you'll need to do is to wrap your code in a transaction, if possible.
Either you don't handle it and it kills the process, or you handle it at the top level of a web server or similar, and it just kills that request (and that doesn't affect other requests), or you've written explicit error handling logic in which case you've hopefully thought about how to handle the error so as not to carry garbage, inconsistent state around, and hopefully also written some tests for it. This gets even easier if you mostly use immutable data, or at least limit shared state between components.
GGP's original point was that "the only safe thing to do [when encountering an exception] is crash" and for managed-memory-languages that is simply not true.
If you can well and truly isolate the state that concerns a single request from any other state, then yes, it is safe. The problem is proving that such isolation holds.
Memory-safe or not doesn't actually matter much here.
If it crashes enough that you'd get terrible reviews it would definitely be caught during development.
If it messes up your mesh silently then you'll definitely still get bad reviews.
And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it.
i don't know in which reality you live but in mine there's not much relationship between the existence of crashes, and them being resolved during development
> And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it.
i don't even know what to say. that's the whole point of destructors - you know that things will be unwound in the reverse order from which they were created on automatic storage. that's, like, why c++ exists
Really? Crashes during development are the easiest thing to resolve aren't they? Your debugger gets triggered automatically. In my experience crashes are treated as highest priority.
> i don't even know what to say. that's the whole point of destructors - you know that things will be unwound in the reverse order from which they were created on automatic storage. that's, like, why c++ exists
Have you met my friend the virtual keyword?
sure but here we're talking about the edge cases which you won't encounter during development. e.g. imagine you're writing a 3D .obj parser and someone somewhere uses a mesh which for some reason has a NaN instead of a real value somewhere, and your parser parses it and gets a NaN into your system - you'll only encounter the subsequent crashes if your tests had expected such a thing.
now multiply this by infinity - it's impossible to think of everything beforehand. But you really don't want the person to crash their 3D software because of that invalid .obj file, just to show them a pop-up "the mesh could not be loaded" and roll-back to before the loading was attempted.
> Have you met my friend the virtual keyword?
.. what about it ? compilers warn you when you forget it if you aren't using GCC 2.95. https://gcc.godbolt.org/z/d4Yj9z3zf
This hasn't been an issue for more than a decade. clang 3.0 (2011) and gcc 4.1 (2007) already warned for this (and there aren't older compilers on CE that I can test).
There are people who start learning programming today who weren't even born when GCC implemented this FFS
The real fun is really the combination with exceptions and destructors. So much so, some projects essentially ban exceptions in C++ code.
[1]: https://blog.burntsushi.net/unwrap/#what-about-when-invarian...
* Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time.
And do... what when the check fails? Can you please write the code for it? Because I don't understand what the heck you mean here. If you don't know Rust, pseudo code is fine.
> Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time.
This is just copying what I already said in the blog post. Until someone can show me how to prove the correctness of arbitrary DFA construction and search from a user provided regular expression pattern and demonstrate its use in a practical programming language like Rust, I consider this a total non-answer, not a "long term" answer.
Basically, your answer here confirms for me that your views on how code should be structured are incoherent.
fn is_match(&self, haystack: &[u8]) -> Result<bool, Error> { ... }So it looks like I already provided a rewrite of the code for you in your preferred style:
// Returns true if the DFA matches the entire 'haystack'.
// This routine always returns either Ok(true) or Ok(false) for all inputs.
// It never returns an error unless there is a bug in its implementation.
fn is_match(&self, haystack: &[u8]) -> Result<bool, &'static str> {
let mut state_id = self.start_id;
for &byte in haystack {
let row = match state_id.checked_mul(256) {
None => return Err("state id too big"),
Some(row) => row,
};
let row_offset = match row.checked_add(usize::from(byte)) {
None => return Err("row index too big"),
Some(row_offset) => row_offset,
};
state_id = match self.transitions.get(row_offset) {
None => return Err("invalid transition"),
Some(&state_id) => state_id,
};
match self.is_match_id.get(state_id) {
None => return Err("invalid state id"),
Some(&true) => return Ok(true),
Some(&false) => {}
}
}
Ok(false)
}
My favorite part is the docs that say "this never returns an error." Because if it did, the code would be buggy. And now all sorts of internal implementation details are leaked.You can't even document the error conditions in a way that is related to the input of the routine, because if you could, you would have discovered a bug!
Another nail in the coffin of your preferred style is that this will absolutely trash the performance of a DFA search loop.
"this will absolutely trash the performance of a DFA search loop."
Do you happen to have some hard data to quantify? I'm curious.
Writeup from the contributor: https://nnethercote.github.io/2022/02/25/how-to-speed-up-the...
> The Decoder trait used for metadata decoding was fallible, using Result throughout. But decoding failures should only happen if something highly unexpected happens (e.g. metadata is corrupted) and on failure the calling code would just abort. This PR changed Decoder to be infallible throughout—panicking immediately instead of panicking slightly later—thus avoiding lots of pointless Result propagation, for wins across many benchmarks of up to 2%.
fn find_fwd_imp<A: Automaton + ?Sized>(
dfa: &A,
input: &Input<'_, '_>,
pre: Option<&'_ dyn Prefilter>,
earliest: bool,
) -> Result<Option<HalfMatch>, MatchError> {
let mut mat = None;
let mut sid = init_fwd(dfa, input)?;
let mut at = input.start();
while at < input.end() {
let byte = match input.haystack().get(at) {
None => return Err(MatchError::GaveUp { offset: at }),
Some(&byte) => byte,
};
sid = dfa.try_next_state(sid, byte)?;
if dfa.is_special_state(sid) {
if dfa.is_match_state(sid) {
let pattern = dfa.match_pattern(sid, 0);
mat = Some(HalfMatch::new(pattern, at));
} else if dfa.is_dead_state(sid) {
return Ok(mat);
}
}
at = match at.checked_add(1) {
None => return Err(MatchError::GaveUp { offset: at }),
Some(at) => at,
};
}
eoi_fwd(dfa, input, &mut sid, &mut mat)?;
Ok(mat)
}
And here's a search (I ran it a few times): $ regex-cli count matches dfa dense @$medium '\w{42}'
parse time: 45.331µs
translate time: 28.767µs
compile nfa time: 9.577558ms
nfa memory: 698148
compile dense dfa time: 516.497476ms
dense dfa memory: 6562848
dense alphabet length: 113
dense stride: 128
search time: 531.808124ms
counts: [2]
Here's code that's safe that will panic when there's a bug: fn find_fwd_imp<A: Automaton + ?Sized>(
dfa: &A,
input: &Input<'_, '_>,
pre: Option<&'_ dyn Prefilter>,
earliest: bool,
) -> Result<Option<HalfMatch>, MatchError> {
let mut mat = None;
let mut sid = init_fwd(dfa, input)?;
let mut at = input.start();
while at < input.end() {
sid = dfa.next_state(sid, input.haystack()[at]);
if dfa.is_special_state(sid) {
if dfa.is_match_state(sid) {
let pattern = dfa.match_pattern(sid, 0);
mat = Some(HalfMatch::new(pattern, at));
} else if dfa.is_dead_state(sid) {
return Ok(mat);
}
}
at += 1;
}
eoi_fwd(dfa, input, &mut sid, &mut mat)?;
Ok(mat)
}
And here's a search with it: $ regex-cli count matches dfa dense @$medium '\w{42}'
parse time: 33.701µs
translate time: 24.264µs
compile nfa time: 9.55935ms
nfa memory: 698148
compile dense dfa time: 550.959698ms
dense dfa memory: 6562848
dense alphabet length: 113
dense stride: 128
search time: 468.932568ms
counts: [2]
So that's 10% right there. We can keep going though. Let's get rid of the bounds checks. We have to use unsafe though. The stakes have risen. Now if there's a bug, we probably won't get a panic. Instead we get UB. Which might lead to all sorts of bad stuff. fn find_fwd_imp<A: Automaton + ?Sized>(
dfa: &A,
input: &Input<'_, '_>,
pre: Option<&'_ dyn Prefilter>,
earliest: bool,
) -> Result<Option<HalfMatch>, MatchError> {
let mut mat = None;
let mut sid = init_fwd(dfa, input)?;
let mut at = input.start();
while at < input.end() {
sid = unsafe {
let byte = *input.haystack().get_unchecked(at);
dfa.next_state_unchecked(sid, byte)
};
if dfa.is_special_state(sid) {
if dfa.is_match_state(sid) {
let pattern = dfa.match_pattern(sid, 0);
mat = Some(HalfMatch::new(pattern, at));
} else if dfa.is_dead_state(sid) {
return Ok(mat);
}
}
at += 1;
}
eoi_fwd(dfa, input, &mut sid, &mut mat)?;
Ok(mat)
}
And now the same search: $ regex-cli count matches dfa dense @$medium '\w{42}'
parse time: 22.511µs
translate time: 26.843µs
compile nfa time: 8.709352ms
nfa memory: 698148
compile dense dfa time: 565.365005ms
dense dfa memory: 6562848
dense alphabet length: 113
dense stride: 128
search time: 456.402963ms
counts: [2]
For a smaller gain.I bet if me and you sat down with a big codebase side-by-side, I could convince you that the style you're proposing is totally unworkable.
Now, whether it's worth it to use unsafe and elide bounds checks is harder. The gain I showed here is small, but it can be bigger in other examples. It depends. But it is certainly less clear. It helps that this is probanly the easiest kind of unsafe to justify, because you just have to make sure the index is in bounds. Other types of unsafe can be quite tricky to justify.
But to your original point, its true that a multiplexing server should probably try to stay up even if there is a bug uncovered in handling a request. But that's precisely why rust allows you to catch panics.
https://doc.rust-lang.org/std/panic/fn.catch_unwind.html
Edit. Re. already existing runtime checks, I'd expect a decent compiler to optimize away redundant checks, i.e. explicitly coded checks vs. checks injected automatically by the compiler, as long as their semantics is identical.
> The Result type is more appropriate to use for *functions that can fail on a regular basis.*
Emphasis mine. The DFA::is_match routine can never fail on any input. Yet, you want to use a 'Result' with it.
Seriously. Please please pretty please read the blog post. You are literally caught in precisely the tangled mess that I tried to untangle in the post. If my post didn't do it for you, then please consider responding to specific points you disagree with in the post.
Edit. For technical geeking around:
* The point is about redundant check elimination, not full check elimination. A decent compiler should be able to eliminate redundant checks, though perhaps the language could offer some intrinsics (was: pragmas) to make this 100% reliable.
* For DFA proofs, perhaps something like https://courses.grainger.illinois.edu/cs373/fa2013/Lectures/... could be a start. A great intern may tackle it over a summer ;)
If you mean "return an error instead," then bzzt again. Read the next section down.
Panic practically everywhere is just not an admissible option, most definitely not for anything that tries to call itself a systems language. Proving that there is no reasonable alternative would be equivalent to claiming the language is unusable.
> and sheer code execution performance is never the bottleneck in a realistic-sized system.
TIL that code execution perf doesn't matter and I've wasted almost a decade of my life focusing on it. Wow I wish you told me sooner!
What a ridiculous comment.
Ironically, that's exactly how you seem when just drop the "Turing completeness" bomb on a discussion about error handling. It's very hard not to see it as a ridiculously childish tantrum, since not only there's no actual argument, it is by definition impossible for it to be relevant to the discussion.
Anyway, I would not be writing this if it wasn't because you keep pumping direct personal attacks to everyone who dares counter your posts, which are already in a terrible tone.
Or, the obvious alternative that I was trying to point out: your analysis is wrong, and therefore, assuming the language is not useless, there must be a proper alternative to "panics everywhere" * that you are not accounting for !
https://lkml.org/lkml/2021/4/14/1099
* This is just to explicitly write the phrase again before it is (again) twisted as calling for a strict prohibition of all non-terminating conditions or some other childish nonsense.
Even unwinding panics (which are exceptions in all but name) can be an alternative, and Rust does support them.
That lkml post has zero relevance here. He's asking for fallible allocations. Which is totally reasonable.
Serious question: did you read the entire blog? If so, can you specifically point out passages you disagree with and why?
EDIT: And seriously, I started out this conversation with a serious comment asking you to elaborate on what you meant by asking something very concrete: how would you change the code example in my blog to conform to your view? You decided to flippantly respond with a non-answer in the first place. Why not go back to the original question I asked and give a real answer? If you don't know Rust, then use pseudo code. We're talking about a dozen lines here. It shouldn't take long.