Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slogger surprisingly hardens console.log() arguments #2517

Closed
warner opened this issue Feb 23, 2021 · 1 comment · Fixed by #2518
Closed

slogger surprisingly hardens console.log() arguments #2517

warner opened this issue Feb 23, 2021 · 1 comment · Fixed by #2518
Assignees
Labels
bug Something isn't working SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Feb 23, 2021

While debugging some #2018 changes that attempted to apply Far() to an object, I ran into some very weird behavior. I tracked it down to the fact that my slogger code was independently calling harden() on the arguments it received. Because vats get a console which also routes to the slogger, and because Far() and Remotable() cannot be called on already-harden()ed objects, my diagnostic logging was changing the state of the objects under observation, which cost me several hours trying to unwind the weirdness.

export function makeSlogger(writeObj) {
const write = writeObj ? e => writeObj(harden(e)) : () => 0;

The fix is simple: remove that harden(). I'm not entirely sure why I included that call.. I try to be in the habit of harden()ing anything before allowing it to leave the function I'm writing, but now I think that's the wrong instinct. Rather, I should harden() only when creating a new object: if someone else handed the object to me, it is their responsibility to harden it, and it would be surprising for me to harden() it for them.

cc @erights to see if I'm updating my instincts correctly

@warner warner added bug Something isn't working SwingSet package: SwingSet labels Feb 23, 2021
@warner warner self-assigned this Feb 23, 2021
warner added a commit that referenced this issue Feb 23, 2021
This causes surprises, especially when debugging.

closes #2517
warner added a commit that referenced this issue Feb 23, 2021
This causes surprises, especially when debugging.

closes #2517
@erights
Copy link
Member

erights commented Feb 23, 2021

I should harden() only when creating a new object: if someone else handed the object to me, it is their responsibility to harden it, and it would be surprising for me to harden() it for them.

cc @erights to see if I'm updating my instincts correctly

Correct. Usually literally correct, but there's one valid pattern where this is correct once we expand the meaning of "create".

Sometimes one need to do some further computed initialization before exposing the object to clients. An object lifecycle is often

  • literally create the object. Keep it closely held.
  • initialize the object. Keep it closely held during initialization.
  • initialization is complete. All invariants are in place. It is ready to be exposed to clients.
  • harden it
  • expose it to clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants