Callback support for libhwbinder.
One of the prime differences between HIDL and Binder
is that HIDL allows the server implementation to do
a synchronous callback into the client with response
data. This callback allows the server to respond with
data that is anywhere - on the stack, the heap or globals -
without worrying about the lifetime of the data, since
it can immediately clean up (if necessary) when the callback
returns.
So we go from a chain like this:
IPCThreadState::executeCommand() // Execute BR_TRANSACTION
BBinder::transact()
ServerStub::onTransact()
Server::some_method()
// Do work, copy data, and return to ServerStub
// serialize data, and return to BBinder
// return to IPCThreadState
IPCThreadState::sendReply()
// done
to a chain like this:
IPCThreadState::executeCommand () // Execute BR_TRANSACTION
BBinder::transact ()
ServerStub::onTransact()
Server::some_method()
// Do work, send reply
ServerStub::TransactCallback(reply)
// Serialize data (until we get scatter-gather)
BBinder::TransactCallback(serialized_reply)
IPCThreadState::sendReply(serialized_reply)
// send reply over binder, return to BBinder
// return to ServerStub
// return to Server, server can clean up
// return to ServerStub
// return to BBinder
// return to IPCThreadState
// IPCThreadState cleanup
// done
To support this, the transact() and onTransact() methods
must support a callback argument, which is added here.
On the proxy side, this callback argument for transact()
can be ignored for now; after all, once the client side gets a
reply, the data is now in the reply Parcel, and the lifetime
of that Parcel is controlled by the Proxy itself. So it
can simply wait for transact() to return, at which point
the reply Parcel is filled.
In the existing Binder model, it would then deserialize
the reply Parcel into arguments that the client had passed
in, and then the Parcel will go out of scope and gets cleaned up.
In HIDL, we deseralize, call the *client* callback, and
then the Parcel goes out of scope and gets cleaned up.
This will change when we get scatter-gather support, but
this allows us to work with the callback-model before that.
Change-Id: If59ee37f68376bc232f3ecbfbe789f7f4d522003
diff --git a/IPCThreadState.cpp b/IPCThreadState.cpp
index 9a7eef3..afdefce 100644
--- a/IPCThreadState.cpp
+++ b/IPCThreadState.cpp
@@ -1094,6 +1094,7 @@
Parcel reply;
status_t error;
+ bool reply_sent = false;
IF_LOG_TRANSACTIONS() {
TextOutput::Bundle _b(alog);
alog << "BR_TRANSACTION thr " << (void*)pthread_self()
@@ -1105,32 +1106,59 @@
<< ", offsets addr="
<< reinterpret_cast<const size_t*>(tr.data.ptr.offsets) << endl;
}
+
+ auto reply_callback = [&] (auto &replyParcel) {
+ if (reply_sent) {
+ // Reply was sent earlier, ignore it.
+ ALOGE("Dropping binder reply, it was sent already.");
+ return;
+ }
+ reply_sent = true;
+ if ((tr.flags & TF_ONE_WAY) == 0) {
+ replyParcel.setError(NO_ERROR);
+ sendReply(replyParcel, 0);
+ } else {
+ ALOGE("Not sending reply in one-way transaction");
+ }
+ };
+
if (tr.target.ptr) {
// We only have a weak reference on the target object, so we must first try to
// safely acquire a strong reference before doing anything else with it.
if (reinterpret_cast<RefBase::weakref_type*>(
tr.target.ptr)->attemptIncStrong(this)) {
error = reinterpret_cast<BBinder*>(tr.cookie)->transact(tr.code, buffer,
- &reply, tr.flags);
+ &reply, tr.flags, reply_callback);
reinterpret_cast<BBinder*>(tr.cookie)->decStrong(this);
} else {
error = UNKNOWN_TRANSACTION;
}
} else {
- error = the_context_object->transact(tr.code, buffer, &reply, tr.flags);
+ error = the_context_object->transact(tr.code, buffer, &reply, tr.flags,
+ reply_callback);
+ }
+
+ if ((tr.flags & TF_ONE_WAY) == 0) {
+ if (!reply_sent) {
+ // Should have been a reply but there wasn't, so there
+ // must have been an error instead.
+ reply.setError(error);
+ sendReply(reply, 0);
+ } else {
+ if (error != NO_ERROR) {
+ ALOGE("transact() returned error after sending reply.");
+ } else {
+ // Ok, reply sent and transact didn't return an error.
+ }
+ }
+ } else {
+ // One-way transaction, don't care about return value or reply.
}
//ALOGI("<<<< TRANSACT from pid %d restore pid %d uid %d\n",
// mCallingPid, origPid, origUid);
-
- if ((tr.flags & TF_ONE_WAY) == 0) {
- LOG_ONEWAY("Sending reply to %d!", mCallingPid);
- if (error < NO_ERROR) reply.setError(error);
- sendReply(reply, 0);
- } else {
- LOG_ONEWAY("NOT sending reply to %d!", mCallingPid);
- }
+
mCallingPid = origPid;
mCallingUid = origUid;
