`outbound_event_bookkeeping` is not implemented for `Event::Sync` #6

New Issue

Open

opened 2024-05-06 10:17:46 +00:00 by tonyg · 2 comments

tonyg commented

2024-05-06 10:17:46 +00:00

Owner

There's a todo!() in the code in relay.rs that needs to be fixed.

[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.

tonyg commented

2024-05-06 10:39:02 +00:00

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.

tonyg commented

2024-05-06 10:44:29 +00:00

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.

No Label

No Milestone

No project

No Assignees

1 Participants

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.

outbound_event_bookkeeping is not implemented for Event::Sync #6

`outbound_event_bookkeeping` is not implemented for `Event::Sync` #6