Fix runtime crash

[bradfitz]

Fixes #2

[bradfitz]

/cc @danderson

[bradfitz]

PTAL @josharian @danderson

[danderson]

Based on my understanding of what the issue was previously, this fix LGTM.

[bradfitz]

Cool. Will submit. @josharian can school us if we're wrong. 😂

[josharian]

I've been pondering this intermittently. I don't understand the handling of refs. It's not actual refcounting. It seems to me e.g. that the finalizer could set refs = 1 before setting a new finalizer, which could speed up things considerably. Is a bool enough?

[danderson]

Yeah, while writing docstrings explaining this interplay, I realized that this is not actually refcounting, but something akin to mark-and-sweep. As currently written, the weakref will need one GC cycle per call to Get before the interned value is actually deleted. So, it does still save memory, but the freeing schedule is unpredictable.

I think Josh is right that in the "re-set finalizer" case, we can set refs=1 to speed things up. In that case, a dangling Value will get collected at most 3 GC cycles after its last reference was dropped. I'll send a patch for same alongside the docstrings explaining this. I welcome a bikeshed for a new name for refs, since it's not a refcount.

[bradfitz]

It seems to me e.g. that the finalizer could set refs = 1 before setting a new finalizer, which could speed up things considerably.

Yeah, that's a good idea.

Is a bool enough?

Empirically, yes. PR coming.