poison database handles on panics

[nikomatsakis]

Continuing the conversation with @kleimkuhler from https://github.com/salsa-rs/salsa/pull/81, I think we should not attempt to make an individual database handle panic safe, but we should instead poision the handles so that -- if they are used again -- you get an immediate panic. You can still recover from panics by using snapshots.

What I wrote in #81 was as follows:

It seems "ok" to say that we recover from panics at the level of snapshots but not within a thread. In fact, this makes some sense -- if the thread with the mutable handle panics, we don't know if all of the input queries etc are in a coherent state.

For example, you might be doing:

db.query_mut(Input1).set(...);
let old_value = db.bar(); // read bar query, which panics unexpectedly
db.query_mut(Input2).set(...); // this will never execute!

However, if we are not going to recover from panics -- and I am now thinking we should not -- then perhaps we want to at least poison the database so we "fail fast". The idea would be to have some panic-guard that sets a "poisoned" flag in the runtime to true when unwinding, and any attempt to use that database handle again will panic quickly with "poisoned".

[nikomatsakis]

@kleimkuhler if you want to see this through, feel free to assign yourself. =)

[nikomatsakis]

Mentoring instructions

We need to add a poisoned flag to LocalState:

https://github.com/salsa-rs/salsa/blob/1af16d5d7c2e30e4438168d4a2077bac695ac164/src/runtime.rs#L401-L405

We need to create a PanicGuard type in execute_query_implementation that will -- upon panic -- set this flag to true (we can use std::mem::forget to prevent the guard from executing upon a normal return):

https://github.com/salsa-rs/salsa/blob/1af16d5d7c2e30e4438168d4a2077bac695ac164/src/runtime.rs#L238-L243

We probably want to check the panic guard flag in that same function and panic if it is true. There may be other places where it makes sense to check, but that's a good starting point.

[nikomatsakis]

I created a Zulip topic for this issue

[kleimkuhler]

Discussion topic

Edit: Moved to https://github.com/salsa-rs/salsa/pull/89

[nikomatsakis]

Actually, we've decided to make the database handles recover from panics, as part of our cancellation mechanism (and, afaik, they currently do). See https://github.com/salsa-rs/salsa/pull/114