clarify buffer lifetime extension applies to all RPCs

Issue #147 resolved
Dan Bonachea created an issue

John wrote:

Another bug is that the rput's rpc has its return value discarded such that if the user were trying to extend its buffer lifetime by returning a future, that future is dropped and the buffer wont live past the rpc scope. Not sure if we even spec this.

Here's what the spec currently says in 6.5-6:

The lifetime of the underlying data buffer and all view iterators on the target in both the
DefinitelyTriviallySerializable and non-DefinitelyTriviallySerializable cases is restricted by
default to the duration of the RPC. In this case, the elements must be processed or copied
elsewhere before the RPC returns. However, if the RPC returns a future, then the lifetime
of the buffer and view iterators is extended until that future is readied. This allows an
RPC to initiate an asynchronous operation to consume the elements, and as long as the
resulting future is returned from the RPC, the underlying buffer will remain alive until the
asynchronous operation is complete and the future readied.

This text is currently not specific to the mechanism used to inject the RPC, but it's confusing since only one of the three mechanisms delivers a return value to the initiator.

It's admittedly "odd" for buffer lifetime extension to apply in the latter two injection cases where there is no semantic acknowledgement of RPC completion to the initiator, so any return value is only seen by the implementation (however one could view the runtime as the "consumer" of the returned future, where "consumption" means freeing the network buffer upon readiness).

We discussed the semantic issue in the 2019-08-28 meeting, and resolved to clarify the spec to guarantee the buffer lifetime extension semantic applies to all RPCs, regardless of the injection mechanism.

@{557058:4d3d8105-7b24-4954-af80-24a77999ba1b}'s report on current implementation status (referring to PR 110):

  1. rpc and rpc_ff use future returns correctly to prolong buffers on both develop and this PR.

  2. remote_cx::as_rpc is and has been brutally broken on develop. Only trivially-copyable function and arguments appear to work. There was a bug (fixed by this PR) where the fun/args were being moved-out-from before being serialized. Move constructors on trivial data usually leave the moved-out-from object unmodified which is why it wasn't biting us for those trivial rpc's, but this is unspecified behavior. And I don't think we ever tested with non-trivial remote_cx::as_rpc's. Adding to that bug, any return from the rpc was being silently dropped, so lifetime extension was not being honored.

  3. remote_cx::as_rpc on this PR fixes the above bug: nontrivial stuff serializes correctly. But return values are still silently dropped, so lifetime extension is again not honored.

This means the spec and implementation currently diverge on view buffer lifetime extension for remote_cx::as_rpc, but we expect to document this compliance issue or possibly fix it before the upcoming release.

Comments (1)

  1. Log in to comment