Hi Kenton,
I think I may be running into some of the cleanup complication you're
implying. During that, I think I'm seeing a straight-up bug, though. The
snippet of code below is from async-win32.c++, just with the comment blocks
removed.
Win32IocpEventPort::~IoPromiseAdapter() {
if (handle != INVALID_HANDLE_VALUE) {
if (!CancelIoEx(handle, this)) {
DWORD error = GetLastError();
if (error != ERROR_NOT_FOUND && error != ERROR_INVALID_HANDLE) {
KJ_FAIL_WIN32("CancelIoEx()", error, handle);
}
// if error was ERROR_INVALID_HANDLE, the handle will be !=
INVALID_HANDLE_VALUE
}
while (handle != INVALID_HANDLE_VALUE) {
port.waitIocp(INFINITE); // this call chokes on an invalid handle
and throws an exception
}
}
}
Here's one possible fix:
~IoPromiseAdapter() {
if (handle != INVALID_HANDLE_VALUE) {
if (!CancelIoEx(handle, this)) {
DWORD error = GetLastError();
if( error == ERROR_INVALID_HANDLE ) {
// we know the handle has gone invalid, so just forget our handle
handle = INVALID_HANDLE_VALUE;
}
else if (error != ERROR_NOT_FOUND ) {
KJ_FAIL_WIN32("CancelIoEx()", error, handle);
}
}
while (handle != INVALID_HANDLE_VALUE) {
port.waitIocp(INFINITE);
}
}
}
If it would help, I can express that in the form of a pull request.
Joe
On Friday, April 19, 2019 at 10:03:32 AM UTC-7, Kenton Varda wrote:
>
> Hi Joe,
>
> Take a look at the doc comments for thisCap():
>
> inline Capability::Client thisCap();
> // Get a capability pointing to this object, much like the `this`
> keyword.
> //
> // The effect of this method is undefined if:
> // - No capability client has been created pointing to this object.
> (This is always the case in
> // the server's constructor.)
> // - The capability client pointing at this object has been destroyed.
> (This is always the case
> // in the server's destructor.)
> // - Multiple capability clients have been created around the same
> server (possible if the server
> // is refcounted, which is not recommended since the client itself
> provides refcounting).
>
> Your code is hitting the first bullet point -- you're trying to call
> thisCap() before any capability client has been created.
>
> What you really want to do here is take advantage of the fact that
> capability clients are copyable using reference counting. You can create a
> client, and then keep a copy for yourself, like so:
>
> auto server = kj::heap<MyTypeImpl>(...);
> auto& serverRef = *server;
> MyType::Client capability = kj::mv(server);
>
> // Send a ref to the client.
> context.getResults().setApp(capability);
>
> // Also keep one for ourself, along with a reference to the
> implementation
> myServers.add(ClientRefPair { client, serverRef });
>
> By keeping a `Client` that points to your server object, you ensure that
> the refcount remains non-zero so the object stays alive. As shown above,
> you can separately keep a reference or pointer to the underlying server
> object if you need to be able to access it's C++ interface directly.
>
> Note that your server's destructor is only called after all references --
> yours, and remote references -- are dropped. If you need to force your
> server object to be destroyed at a specific time, then you'll need to do
> one of the following:
>
> - Use a layer of indirection where your server object wraps another object
> that can be nulled out later, such that all the RPC methods start throwing
> exceptions once nulled.
> - Wrap the Client object in a membrane (capnp/membrane.h) which you later
> revoke.
>
> -Kenton
>
> On Thu, Apr 18, 2019 at 10:00 PM Joe Ludwig <[email protected]
> <javascript:>> wrote:
>
>> I have a really simple RPC protocol:
>> interface AvServer
>> {
>> createApp @0 ( name: Text ) -> ( app: AvApp );
>> }
>>
>>
>> interface AvApp
>> {
>> name @0 () -> ( name: Text );
>>
>>
>> destroy @1 ();
>> }
>>
>>
>> I implement that on the server with two concrete classes:
>>
>> class AvServerImpl final : public AvServer::Server
>> class CAardvarkApp final: public AvApp::Server
>>
>>
>>
>> And then when the client calls createApp I want to create an AvApp,
>> return a reference to it, and keep ownership of the app itself in some
>> datastructure on the server:
>>
>> ::kj::Promise<void> AvServerImpl::createApp( CreateAppContext context )
>> {
>> // "works"
>> context.getResults().setApp( kj::heap<CAardvarkApp>( context.getParams
>> ().getName() ) );
>>
>>
>> // crashes because "thisHook" is null
>> auto pApp = kj::heap<CAardvarkApp>( context.getParams().getName() );
>> context.getResults().setApp( pApp->GetClient() ); // GetClient() is
>> just a call to thisCap()
>> m_listApps.push_back( std::move( pApp ) );
>>
>>
>> return kj::READY_NOW;
>> }
>>
>>
>> The first approach there works, but the AvServerImpl class doesn't keep
>> any kind of record of the new CAardvarkApp object, so it won't be able to
>> do anything with the app outside of a call from this one particular client.
>>
>> What I'm attempting to do in the second approach is create an object on
>> the heap and set it in the results, but keep ownership of it in the list. I
>> don't get that far, though, because thisCap crashes because thisHook
>> returns nullptr.
>>
>> I think I must be missing something really fundamental about how all this
>> is supposed to work. Am I required to create a unique foo::Server object
>> for each incoming call that refers to a single logical object? If I have to
>> pass ownership to the results in order to return values, it seems like
>> something along that line will be called for. That doesn't sound right,
>> though, since I'd have to manage an unbounded number of those objects as
>> requests come in from different clients.
>>
>> Help? :)
>>
>>
>> Joe
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Cap'n Proto" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected] <javascript:>.
>> Visit this group at https://groups.google.com/group/capnproto.
>>
>
--
You received this message because you are subscribed to the Google Groups
"Cap'n Proto" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
Visit this group at https://groups.google.com/group/capnproto.
