outbound_event_bookkeeping is not implemented for Event::Sync #6

Open
opened 2024-05-06 10:17:46 +00:00 by tonyg · 2 comments
Owner
[There's a `todo!()` in the code in `relay.rs`](https://git.syndicate-lang.org/syndicate-lang/syndicate-rs/src/commit/6468e16790dfe6865780bf942af2412dbb0b4f32/syndicate/src/relay.rs#L534) that needs to be fixed.
Author
Owner

The purpose of outbound_event_bookkeeping is to update entity reference counts across the membranes of the active relay as events flow outward toward the peer.

  • an assert increments the reference count on the wiresymbol denoting the target; refcount increments have already been done as part of encoding the packet for transmission.
  • a message is transient, so doesn't increment the reference count on the target, and furthermore decrements the refcounts that the encoding process just incremented. If a refcount hits zero as part of this, that's an error, because the user just sent a transient reference.
  • a retract decrements the increments made when the corresponding assert happened; the encoding process of the packet holding the retract doesn't change any refcounts itself, since it just carries a handle.
  • the code for sync currently just calls todo!().

So, what should it do?

  • a sync implicitly references a target, so it should increment the refcount of the target, just like assert
  • a sync carries a reference to a local entity, the reply-continuation more-or-less. The encoding process will have already incremented the refcount of this entity.

That's fairly straightforward-seeming. So why the todo!() rather than just implementing it? The problem, I think, is that there's nowhere to put the incref'd handle such that when the sync response comes in it gets released again.

Other implementations usually supply a SyncPeerEntity thing which knows to drop both itself and the target entity from the membrane's indexes when a message comes in. In the Rust code, there's a SyncPeer for the inbound side, so I think we need the symmetric outbound kind of proxy.

Edited to add: Unfortunately by the time outbound_event_bookkeeping is called, the sync with the original, non-wrapped reply-continuation entity has already been encoded and transmitted. We need to wrap the entity ref with some kind of SyncPeerEntity before it gets transmitted.

The purpose of `outbound_event_bookkeeping` is to update entity reference counts across the membranes of the active relay as events flow outward toward the peer. - an `assert` increments the reference count on the wiresymbol denoting the target; refcount increments have already been done as part of encoding the packet for transmission. - a `message` is transient, so doesn't increment the reference count on the target, and furthermore *decrements* the refcounts that the encoding process just incremented. If a refcount hits zero as part of this, that's an error, because the user just sent a transient reference. - a `retract` decrements the increments made when the corresponding `assert` happened; the encoding process of the packet holding the `retract` doesn't change any refcounts itself, since it just carries a handle. - the code for `sync` currently just calls `todo!()`. So, what *should* it do? - a sync implicitly references a target, so it should increment the refcount of the target, just like `assert` - a sync carries a reference to a local entity, the reply-continuation more-or-less. The encoding process will have already incremented the refcount of this entity. That's fairly straightforward-seeming. So why the `todo!()` rather than just implementing it? The problem, I think, is that there's nowhere to put the incref'd handle such that when the sync response comes in it gets released again. Other implementations usually supply a `SyncPeerEntity` thing which knows to drop both itself and the target entity from the membrane's indexes when a message comes in. In the Rust code, there's a `SyncPeer` for the *inbound* side, so I think we need the symmetric outbound kind of proxy. **Edited to add:** Unfortunately by the time `outbound_event_bookkeeping` is called, the sync with the original, non-wrapped reply-continuation entity has already been encoded and transmitted. We need to wrap the entity ref with some kind of `SyncPeerEntity` *before* it gets transmitted.
Author
Owner

A good place to do all this would be in impl Entity<AnyValue> for RelayEntity. Perhaps it could do the necessary target incref, too, leaving the actual location of the todo!() a no-op.

A good place to do all this would be in `impl Entity<AnyValue> for RelayEntity`. Perhaps it could do the necessary target incref, too, leaving the actual location of the `todo!()` a no-op.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: syndicate-lang/syndicate-rs#6
No description provided.