Mysterious SES_ALREADY_LOCKED_DOWN failure when using @endo/init/debug.js

[erights]

agoric-sdk/packages/cosmic-swingset/test/test-clean-core-eval.js

Line 1 in 09b239d

import '@endo/init';

Changing this to '@endo/init/debug.js' but that fails with SES_ALREADY_LOCKED_DOWN for reasons we have not yet diagnosed.

See #5567

[kriskowal]

I’ve isolated the root cause. If the test imports @endo/init/debug.js, it then also imports ../scripts/clean-core-eval.js, which in turn imports @endo/init, resulting in another lockdown() call.

We can fix this immediately by changing both to debug.js. This would mean the script runs in debug mode. I’ll defer to @michaelfig whether that’s acceptable. If it’s not acceptable, we would need to factor debugEvaluableCode and compareEvaluate into shared library code.

[erights]

Does this mean that lockdown was called twice, and would continue to be called twice? I thought we changed lockdown to unconditionally reject that.

[michaelfig]

I’ll defer to @michaelfig whether that’s acceptable.

I believe the correct solution is to introduce state in the lockdown wrapper in @endo/lockdown/pre.js to only actually lock down the first time. I recognize that wasn't desirable behaviour for ses proper, but I think it's useful to ensure that the @endo/init and @endo/lockdown wrappers never cause errors of this kind.

[kriskowal]

We do unconditionally reject calling lockdown again, and this issue illustrates that failure mode.

I’m weary of accidentally downgrading lockdown in an import-order dependent way and I do think this error has guided us to improve the software. I agree that the error, as presented here, is a bad developer experience. The underlying error reveals the stack at both points at lockdown, and ava occluded the stacks, even with verbose stack filtering.

To debug this, I instrumented a log with both the stack and PID into the SES version in the working copy, which is not something I’d expect a downstream developer to be able to do. However, we could include instructions in the error message or linked error code documentation to use another lockdown option to unconditionally log the stack and PID at lockdown.

[michaelfig]

I think I have a recommendation. The current behaviour (fail fast if already locked down by anything else) should be implemented by@endo/init/lockdown.js. Then, the default @endo/init should be idempotent: a no-op if already locked down by anything else in @endo/init (like @endo/init/debug.js or @endo/init/lockdown.js).