Audit context guard soundness about chakracore-rs HOT 2 OPEN

Sorry for the delay.

That's a valid observation. Since only one context may be active at a time (per thread), a probable solution would be to use a thread-local stack instead of each context tracking their predecessor.

EDIT: Although that does not guarantee soundness either, but I believe I have a different solution.

from chakracore-rs.

I've pondered more about a potential solution and each have their own pros and cons

• Use a stack-based context logic and associate each guard with an ID:

  Context	ID
  Context1	1234
  Context2	5234
  Context3	1236

  Whenever a guard is dropped, it checks whether it is currently active, and if so pops itself from the
  thread-local stack and activates its predecessor. Otherwise, if not currently active, it just removes
  itself from the stack by its ID. This would also require a reentrant mutex for each runtime (unless this validation is deferred to ChakraCore itself).

  Pros	Cons
  Simple API	Requires reentrant mutex for the runtime
  -	Introduces guard inconsistencies*

  *A guard may not ensure that the context it references is actually active.

  let guard1 = context1.make_current()?;
  let guard2 = context2.make_current()?;
  
  // This property would still be associated with `context2`
  let property = Property::new(&guard1, "foo"); 

• Take advantage of the runtime being Send + Sync, and expose a &mut self for activating a context (where only one context can be activate at a time).

  This approach would rely on the type system to ensure that the runtime is only used from a single thread at a time, whilst also preventing guard inconsistencies.

  Pros	Cons
  No guard inconsistencies	Arguably a more cumbersome API
  No reentrant mutex	-

  let guard1 = runtime.activate_context(&context1);
  
  // The following would not compile since `runtime` is already mutably borrowed
  let guard2 = runtime.activate_context(&context2);

  The downside is of course that the runtime must be passed around whenever a context is used.

  NOTE: An extension to this method may also be to allow nested guards:

  let guard2 = guard1.switch(&context2);
  
  // The following would not compile since `guard1` is mutably borrowed during the `switch`
  let property = Property::new(&guard1, "foo"); 

It may also be worth mentioning that none of these options would solve scenarios where values are intermixed from different contexts:

let object1 = Object::new(&guard1);
let guard2 = guard1.switch(&context2);

// `object1` is associated with `context1`, whilst a value from `context2` is assigned.
object1.set(&guard2, &property, Number::new(&guard2, 10));

This may be solved by each value tracking its own associated context, but I'm not sure it's worth the overhead.

I'd love some input if anyone is interested or has any ideas. I will continue to ponder but I believe using runtime + &mut may be the cleanest and most robust solution.

from chakracore-rs.