-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Store ClientSnapshots in the CapTable. #525
Conversation
I plan on adding *Snapshot versions and eventually storing snapshots internally.
This seems like an unnecessary complication, and I don't want to deal with it when porting stuff over to snapshots.
...and use it to catch a bug; the test is failing, need to track it down.
Otherwise the target message steals the caps, resulting in errors later when we try to read them.
Per capnproto#523, this is currently failing frequently. Mark it as flaky until we're ready to actually deal with it.
In addition to letting us hand out borrowed references, Storing the snapshots will facilitate code that needs them not to resolve further after AddSnapshot() or SetSnapshot().
Marking this as a draft; fussing with this on one of the follow-up features has convinced me that this is a bit error prone: you end up with two things to remember to release, and I'm hitting a cascade of leaks in branch for #480. I may rethink how to do this, time to sleep on it. |
I think instead of doing this, I'm going to leave |
Stacked on #522 and #524; review and merge those first. This makes the CapTable hang on to
ClientSnapshots
directly, which will be important for addressing the 4-way race in the context of answers.