EDK: Make TransportData preserve handle rights.
Now, at least theoretically, handle rights should be preserved when sent
over a message pipe (including across processes). (I'll add/update tests
separately.)
R=azani@chromium.org, vardhan@google.com
Review URL: https://codereview.chromium.org/1956843002 .
diff --git a/mojo/edk/system/channel.cc b/mojo/edk/system/channel.cc
index b52cba6..7ce9e76 100644
--- a/mojo/edk/system/channel.cc
+++ b/mojo/edk/system/channel.cc
@@ -408,7 +408,7 @@
std::unique_ptr<MessageInTransit> message(new MessageInTransit(message_view));
if (message_view.transport_data_buffer_size() > 0) {
DCHECK(message_view.transport_data_buffer());
- message->SetDispatchers(TransportData::DeserializeDispatchers(
+ message->SetHandles(TransportData::DeserializeHandles(
message_view.transport_data_buffer(),
message_view.transport_data_buffer_size(), std::move(platform_handles),
this));
diff --git a/mojo/edk/system/message_in_transit.cc b/mojo/edk/system/message_in_transit.cc
index 4dbcf7d..c40b9f5 100644
--- a/mojo/edk/system/message_in_transit.cc
+++ b/mojo/edk/system/message_in_transit.cc
@@ -10,6 +10,7 @@
#include "base/logging.h"
#include "mojo/edk/system/configuration.h"
+#include "mojo/edk/system/dispatcher.h"
#include "mojo/edk/system/transport_data.h"
using mojo::platform::AlignedAlloc;
@@ -155,19 +156,6 @@
#endif
}
-void MessageInTransit::SetDispatchers(
- std::unique_ptr<DispatcherVector> dispatchers) {
- DCHECK(dispatchers);
-
- std::unique_ptr<HandleVector> handles(new HandleVector());
- handles->reserve(dispatchers->size());
- for (size_t i = 0; i < dispatchers->size(); i++) {
- handles->push_back(
- Handle(std::move(dispatchers->at(i)), MOJO_HANDLE_RIGHT_NONE));
- }
- SetHandles(std::move(handles));
-}
-
void MessageInTransit::SetTransportData(
std::unique_ptr<TransportData> transport_data) {
DCHECK(transport_data);
diff --git a/mojo/edk/system/message_in_transit.h b/mojo/edk/system/message_in_transit.h
index f2ec026..654311e 100644
--- a/mojo/edk/system/message_in_transit.h
+++ b/mojo/edk/system/message_in_transit.h
@@ -14,7 +14,6 @@
#include "mojo/edk/platform/aligned_alloc.h"
#include "mojo/edk/system/channel_endpoint_id.h"
-#include "mojo/edk/system/dispatcher.h"
#include "mojo/edk/system/handle.h"
#include "mojo/edk/system/memory.h"
#include "mojo/public/cpp/system/macros.h"
@@ -189,8 +188,6 @@
// handle in the handle table), i.e., the dispatcher must have a reference
// count of 1. This message must not already have handles.
void SetHandles(std::unique_ptr<HandleVector> handles);
- // TODO(vtl): Delete this.
- void SetDispatchers(std::unique_ptr<DispatcherVector> dispatchers);
// Sets the |TransportData| for this message. This should only be done when
// there are no handles and no existing |TransportData|.
diff --git a/mojo/edk/system/message_pipe_endpoint.h b/mojo/edk/system/message_pipe_endpoint.h
index 65462ea..837e660 100644
--- a/mojo/edk/system/message_pipe_endpoint.h
+++ b/mojo/edk/system/message_pipe_endpoint.h
@@ -11,6 +11,7 @@
#include <vector>
#include "mojo/edk/system/handle.h"
+#include "mojo/edk/system/handle_signals_state.h"
#include "mojo/edk/system/memory.h"
#include "mojo/edk/system/message_in_transit.h"
#include "mojo/public/c/system/handle.h"
diff --git a/mojo/edk/system/message_pipe_test_utils.h b/mojo/edk/system/message_pipe_test_utils.h
index 23a96f6..6f2ebdf 100644
--- a/mojo/edk/system/message_pipe_test_utils.h
+++ b/mojo/edk/system/message_pipe_test_utils.h
@@ -9,6 +9,7 @@
#include "mojo/edk/platform/scoped_platform_handle.h"
#include "mojo/edk/system/channel.h"
+#include "mojo/edk/system/handle_signals_state.h"
#include "mojo/edk/system/test/test_io_thread.h"
#include "mojo/edk/test/multiprocess_test_helper.h"
#include "mojo/edk/util/ref_ptr.h"
diff --git a/mojo/edk/system/transport_data.cc b/mojo/edk/system/transport_data.cc
index 058d795..05a03ba 100644
--- a/mojo/edk/system/transport_data.cc
+++ b/mojo/edk/system/transport_data.cc
@@ -19,22 +19,6 @@
namespace mojo {
namespace system {
-namespace {
-
-// TODO(vtl): Temporary, until |TransportData| really supports handles.
-std::unique_ptr<DispatcherVector> DispatcherVectorFromHandleVector(
- std::unique_ptr<HandleVector> handles) {
- DCHECK(handles);
-
- std::unique_ptr<DispatcherVector> dispatchers(new DispatcherVector());
- dispatchers->reserve(handles->size());
- for (size_t i = 0; i < handles->size(); i++)
- dispatchers->push_back(std::move(handles->at(i).dispatcher));
- return dispatchers;
-}
-
-} // namespace
-
// The maximum amount of space needed per platform handle.
// (|{Channel,RawChannel}::GetSerializedPlatformHandleSize()| should always
// return a value which is at most this. This is only used to calculate
@@ -79,19 +63,13 @@
"alignment");
};
-// TODO(vtl): Make this the real one.
TransportData::TransportData(std::unique_ptr<HandleVector> handles,
Channel* channel)
- : TransportData(DispatcherVectorFromHandleVector(std::move(handles)),
- channel) {}
-
-TransportData::TransportData(std::unique_ptr<DispatcherVector> dispatchers,
- Channel* channel)
: buffer_size_() {
- DCHECK(dispatchers);
+ DCHECK(handles);
DCHECK(channel);
- const size_t num_handles = dispatchers->size();
+ const size_t num_handles = handles->size();
DCHECK_GT(num_handles, 0u);
// The offset to the start of the (Mojo) handle table.
@@ -108,11 +86,12 @@
std::vector<size_t> all_max_platform_handles(num_handles);
#endif
for (size_t i = 0; i < num_handles; i++) {
- if (Dispatcher* dispatcher = (*dispatchers)[i].get()) {
+ if (handles->at(i)) {
size_t max_size = 0;
size_t max_platform_handles = 0;
Dispatcher::TransportDataAccess::StartSerialize(
- dispatcher, channel, &max_size, &max_platform_handles);
+ handles->at(i).dispatcher.get(), channel, &max_size,
+ &max_platform_handles);
DCHECK_LE(max_size, kMaxSerializedDispatcherSize);
estimated_size += MessageInTransit::RoundUpMessageAlignment(max_size);
@@ -159,8 +138,8 @@
buffer_.get() + handle_table_start_offset);
size_t current_offset = serialized_dispatcher_start_offset;
for (size_t i = 0; i < num_handles; i++) {
- Dispatcher* dispatcher = (*dispatchers)[i].get();
- if (!dispatcher) {
+ Handle& handle = handles->at(i);
+ if (!handle) {
static_assert(static_cast<int32_t>(Dispatcher::Type::UNKNOWN) == 0,
"Value of Dispatcher::Type::UNKNOWN must be 0");
continue;
@@ -174,12 +153,12 @@
void* destination = buffer_.get() + current_offset;
size_t actual_size = 0;
if (Dispatcher::TransportDataAccess::EndSerializeAndClose(
- dispatcher, channel, destination, &actual_size,
+ handle.dispatcher.get(), channel, destination, &actual_size,
platform_handles_.get())) {
- handle_table[i].type = static_cast<int32_t>(dispatcher->GetType());
+ handle_table[i].type = static_cast<int32_t>(handle.dispatcher->GetType());
handle_table[i].offset = static_cast<uint32_t>(current_offset);
handle_table[i].size = static_cast<uint32_t>(actual_size);
-// (Okay to not set |unused| since we cleared the entire buffer.)
+ handle_table[i].rights = handle.rights;
#if DCHECK_IS_ON()
DCHECK_LE(actual_size, all_max_sizes[i]);
@@ -212,8 +191,6 @@
// There's no aligned realloc, so it's no good way to release unused space (if
// we overshot our estimated space requirements).
buffer_size_ = current_offset;
-
- // |dispatchers_| will be destroyed as it goes out of scope.
}
TransportData::TransportData(
@@ -331,7 +308,7 @@
}
// static
-std::unique_ptr<DispatcherVector> TransportData::DeserializeDispatchers(
+std::unique_ptr<HandleVector> TransportData::DeserializeHandles(
const void* buffer,
size_t buffer_size,
std::unique_ptr<std::vector<ScopedPlatformHandle>> platform_handles,
@@ -342,8 +319,7 @@
const Header* header = static_cast<const Header*>(buffer);
const size_t num_handles = header->num_handles;
- std::unique_ptr<DispatcherVector> dispatchers(
- new DispatcherVector(num_handles));
+ std::unique_ptr<HandleVector> handles(new HandleVector(num_handles));
const HandleTableEntry* handle_table =
reinterpret_cast<const HandleTableEntry*>(
@@ -357,11 +333,13 @@
DCHECK_LE(offset + size, buffer_size);
const void* source = static_cast<const char*>(buffer) + offset;
- (*dispatchers)[i] = Dispatcher::TransportDataAccess::Deserialize(
- channel, handle_table[i].type, source, size, platform_handles.get());
+ (*handles)[i] = Handle(Dispatcher::TransportDataAccess::Deserialize(
+ channel, handle_table[i].type, source, size,
+ platform_handles.get()),
+ handle_table[i].rights);
}
- return dispatchers;
+ return handles;
}
} // namespace system
diff --git a/mojo/edk/system/transport_data.h b/mojo/edk/system/transport_data.h
index 8698a6d..310e33d 100644
--- a/mojo/edk/system/transport_data.h
+++ b/mojo/edk/system/transport_data.h
@@ -13,6 +13,8 @@
#include "mojo/edk/platform/aligned_alloc.h"
#include "mojo/edk/platform/scoped_platform_handle.h"
#include "mojo/edk/system/dispatcher.h"
+#include "mojo/edk/system/handle.h"
+#include "mojo/public/c/system/handle.h"
#include "mojo/public/cpp/system/macros.h"
namespace mojo {
@@ -87,9 +89,6 @@
static size_t GetMaxPlatformHandles();
TransportData(std::unique_ptr<HandleVector> handles, Channel* channel);
- // TODO(vtl): Remove this, once |TransportData| really supports handles.
- TransportData(std::unique_ptr<DispatcherVector> dispatchers,
- Channel* channel);
// This is used for users of |MessageInTransit|/|TransportData|/|RawChannel|
// that want to simply transport data and platform handles, and not
@@ -138,10 +137,10 @@
size_t* num_platform_handles,
const void** platform_handle_table);
- // Deserializes dispatchers from the given (serialized) transport data buffer
+ // Deserializes handles from the given (serialized) transport data buffer
// (typically from a |MessageInTransit::View|) and vector of platform handles.
// |buffer| should be non-null and |buffer_size| should be nonzero.
- static std::unique_ptr<DispatcherVector> DeserializeDispatchers(
+ static std::unique_ptr<HandleVector> DeserializeHandles(
const void* buffer,
size_t buffer_size,
std::unique_ptr<std::vector<platform::ScopedPlatformHandle>>
@@ -168,7 +167,7 @@
int32_t type; // From |Dispatcher::Type| (|UNKNOWN| for "invalid").
uint32_t offset; // Relative to the start of the "secondary buffer".
uint32_t size; // (Not including any padding.)
- uint32_t unused;
+ MojoHandleRights rights;
};
const Header* header() const {
