Skip to content

Commit

Permalink
PNaCl: Use Chrome IPC to talk to the translator process, instead of SRPC
Browse files Browse the repository at this point in the history 
This is the last remaining use of SRPC.  (I'll remove the remaining
SRPC plumbing in follow-up changes.)

Change pnacl_translate_thread.cc to send its request using Chrome IPC
instead of SRPC.  Similarly, change irt_pnacl_translator_compile.cc to
receive its request this way.

 * Generalise GetHandleForSubprocess() to work for this other process.

 * Use std::string instead of std::vector<char> to avoid needing to
   make an extra copy when creating
   PpapiMsg_PnaclTranslatorCompileChunk.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

Review URL: https://codereview.chromium.org/1564903002

Cr-Commit-Position: refs/heads/master@{#368312}
  • Loading branch information
mseaborn authored and Commit bot committed Jan 8, 2016
1 parent 2bff3b2 commit 98d206c
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 184 deletions.
156 changes: 69 additions & 87 deletions components/nacl/renderer/plugin/pnacl_translate_thread.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/logging.h"
#include "components/nacl/renderer/plugin/plugin.h"
#include "components/nacl/renderer/plugin/plugin_error.h"
#include "components/nacl/renderer/plugin/srpc_params.h"
#include "components/nacl/renderer/plugin/temporary_file.h"
#include "components/nacl/renderer/plugin/utility.h"
#include "content/public/common/sandbox_init.h"
Expand Down Expand Up @@ -74,6 +73,7 @@ PnaclTranslateThread::PnaclTranslateThread()
nexe_file_(NULL),
coordinator_error_info_(NULL),
coordinator_(NULL),
compiler_channel_peer_pid_(base::kNullProcessId),
ld_channel_peer_pid_(base::kNullProcessId) {
NaClXMutexCtor(&subprocess_mu_);
NaClXMutexCtor(&cond_mu_);
Expand Down Expand Up @@ -114,12 +114,18 @@ void PnaclTranslateThread::RunCompile(
DCHECK(compiler_subprocess_->service_runtime());
compiler_subprocess_active_ = true;

// Free this IPC channel now to make sure that it does not get freed on
// the child thread when the child thread calls Shutdown().
// TODO(mseaborn): Convert DoCompile() to using Chrome IPC instead of SRPC,
// the same way DoLink() has been converted. Then we will use this IPC
// channel instead of just freeing it here.
compiler_subprocess_->service_runtime()->TakeTranslatorChannel();
// Take ownership of this IPC channel to make sure that it does not get
// freed on the child thread when the child thread calls Shutdown().
compiler_channel_ =
compiler_subprocess_->service_runtime()->TakeTranslatorChannel();
// compiler_channel_ is a IPC::SyncChannel, which is not thread-safe and
// cannot be used directly by the child thread, so create a
// SyncMessageFilter which can be used by the child thread.
compiler_channel_filter_ = compiler_channel_->CreateSyncMessageFilter();
// Make a copy of the process ID, again to avoid any thread-safety issues
// involved in accessing compiler_subprocess_ on the child thread.
compiler_channel_peer_pid_ =
compiler_subprocess_->service_runtime()->get_process_id();

compile_finished_callback_ = compile_finished_callback;
translate_thread_.reset(new NaClThread);
Expand Down Expand Up @@ -177,7 +183,7 @@ void PnaclTranslateThread::RunLink() {
void PnaclTranslateThread::PutBytes(const void* bytes, int32_t count) {
CHECK(bytes != NULL);
NaClXMutexLock(&cond_mu_);
data_buffers_.push_back(std::vector<char>());
data_buffers_.push_back(std::string());
data_buffers_.back().insert(data_buffers_.back().end(),
static_cast<const char*>(bytes),
static_cast<const char*>(bytes) + count);
Expand All @@ -193,13 +199,13 @@ void PnaclTranslateThread::EndStream() {
}

ppapi::proxy::SerializedHandle PnaclTranslateThread::GetHandleForSubprocess(
TempFile* file, int32_t open_flags) {
TempFile* file, int32_t open_flags, base::ProcessId peer_pid) {
IPC::PlatformFileForTransit file_for_transit;

#if defined(OS_WIN)
if (!content::BrokerDuplicateHandle(
file->GetFileHandle(),
ld_channel_peer_pid_,
peer_pid,
&file_for_transit,
0, // desired_access is 0 since we're using DUPLICATE_SAME_ACCESS.
DUPLICATE_SAME_ACCESS)) {
Expand Down Expand Up @@ -230,29 +236,19 @@ void PnaclTranslateThread::DoCompile() {
// and now, just leave now.
if (!compiler_subprocess_active_)
return;
// Now that we are in helper thread, we can do the the blocking
// StartSrpcServices operation.
if (!compiler_subprocess_->StartSrpcServices()) {
TranslateFailed(
PP_NACL_ERROR_SRPC_CONNECTION_FAIL,
"SRPC connection failure for " + compiler_subprocess_->description());
return;
}
}

SrpcParams params;
std::vector<nacl::DescWrapper*> compile_out_files;
size_t i;
for (i = 0; i < obj_files_->size(); i++)
compile_out_files.push_back((*obj_files_)[i]->write_wrapper());
for (; i < PnaclCoordinator::kMaxTranslatorObjectFiles; i++)
compile_out_files.push_back(invalid_desc_wrapper_);
std::vector<ppapi::proxy::SerializedHandle> compiler_output_files;
for (TempFile* obj_file : *obj_files_) {
compiler_output_files.push_back(
GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_WRITE,
compiler_channel_peer_pid_));
}

PLUGIN_PRINTF(("DoCompile using subzero: %d\n", pnacl_options_->use_subzero));

pp::Core* core = pp::Module::Get()->core();
int64_t do_compile_start_time = NaClGetTimeOfDayMicroseconds();
bool init_success;

std::vector<std::string> args;
if (pnacl_options_->use_subzero) {
Expand All @@ -266,34 +262,20 @@ void PnaclTranslateThread::DoCompile() {
architecture_attributes_);
}

std::vector<char> split_args;
for (const std::string& arg : args) {
std::copy(arg.begin(), arg.end(), std::back_inserter(split_args));
split_args.push_back('\x00');
bool success = false;
std::string error_str;
if (!compiler_channel_filter_->Send(
new PpapiMsg_PnaclTranslatorCompileInit(
num_threads_, compiler_output_files, args, &success, &error_str))) {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Compile stream init failed: "
"reply not received from PNaCl translator "
"(it probably crashed)");
return;
}

init_success = compiler_subprocess_->InvokeSrpcMethod(
"StreamInitWithSplit", "ihhhhhhhhhhhhhhhhC", &params, num_threads_,
compile_out_files[0]->desc(), compile_out_files[1]->desc(),
compile_out_files[2]->desc(), compile_out_files[3]->desc(),
compile_out_files[4]->desc(), compile_out_files[5]->desc(),
compile_out_files[6]->desc(), compile_out_files[7]->desc(),
compile_out_files[8]->desc(), compile_out_files[9]->desc(),
compile_out_files[10]->desc(), compile_out_files[11]->desc(),
compile_out_files[12]->desc(), compile_out_files[13]->desc(),
compile_out_files[14]->desc(), compile_out_files[15]->desc(),
&split_args[0], split_args.size());
if (!init_success) {
if (compiler_subprocess_->srpc_client()->GetLastError() ==
NACL_SRPC_RESULT_APP_ERROR) {
// The error message is only present if the error was returned from llc
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
std::string("Stream init failed: ") +
std::string(params.outs()[0]->arrays.str));
} else {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Stream init internal error");
}
if (!success) {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
std::string("Stream init failed: ") + error_str);
return;
}
PLUGIN_PRINTF(("PnaclCoordinator: StreamInit successful\n"));
Expand All @@ -308,50 +290,48 @@ void PnaclTranslateThread::DoCompile() {
")\n",
done_, data_buffers_.size()));
if (data_buffers_.size() > 0) {
std::vector<char> data;
std::string data;
data.swap(data_buffers_.front());
data_buffers_.pop_front();
NaClXMutexUnlock(&cond_mu_);
PLUGIN_PRINTF(("StreamChunk\n"));
if (!compiler_subprocess_->InvokeSrpcMethod("StreamChunk", "C", &params,
&data[0], data.size())) {
if (compiler_subprocess_->srpc_client()->GetLastError() !=
NACL_SRPC_RESULT_APP_ERROR) {
// If the error was reported by the translator, then we fall through
// and call StreamEnd, which returns a string describing the error,
// which we can then send to the Javascript console. Otherwise just
// fail here, since the translator has probably crashed or asserted.
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Compile stream chunk failed. "
"The PNaCl translator has probably crashed.");
return;
}

if (!compiler_channel_filter_->Send(
new PpapiMsg_PnaclTranslatorCompileChunk(data, &success))) {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Compile stream chunk failed: "
"reply not received from PNaCl translator "
"(it probably crashed)");
return;
}
if (!success) {
// If the error was reported by the translator, then we fall through
// and call PpapiMsg_PnaclTranslatorCompileEnd, which returns a string
// describing the error, which we can then send to the Javascript
// console.
break;
} else {
PLUGIN_PRINTF(("StreamChunk Successful\n"));
core->CallOnMainThread(
0,
coordinator_->GetCompileProgressCallback(data.size()),
PP_OK);
}
PLUGIN_PRINTF(("StreamChunk Successful\n"));
core->CallOnMainThread(
0,
coordinator_->GetCompileProgressCallback(data.size()),
PP_OK);
} else {
NaClXMutexUnlock(&cond_mu_);
}
}
PLUGIN_PRINTF(("PnaclTranslateThread done with chunks\n"));
// Finish llc.
if (!compiler_subprocess_->InvokeSrpcMethod("StreamEnd", std::string(),
&params)) {
PLUGIN_PRINTF(("PnaclTranslateThread StreamEnd failed\n"));
if (compiler_subprocess_->srpc_client()->GetLastError() ==
NACL_SRPC_RESULT_APP_ERROR) {
// The error string is only present if the error was sent back from llc.
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
params.outs()[3]->arrays.str);
} else {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Compile StreamEnd internal error");
}
if (!compiler_channel_filter_->Send(
new PpapiMsg_PnaclTranslatorCompileEnd(&success, &error_str))) {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL,
"Compile stream end failed: "
"reply not received from PNaCl translator "
"(it probably crashed)");
return;
}
if (!success) {
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_INTERNAL, error_str);
return;
}
compile_time_ = NaClGetTimeOfDayMicroseconds() - do_compile_start_time;
Expand Down Expand Up @@ -399,11 +379,13 @@ void PnaclTranslateThread::DoLink() {
}

ppapi::proxy::SerializedHandle nexe_file =
GetHandleForSubprocess(nexe_file_, PP_FILEOPENFLAG_WRITE);
GetHandleForSubprocess(nexe_file_, PP_FILEOPENFLAG_WRITE,
ld_channel_peer_pid_);
std::vector<ppapi::proxy::SerializedHandle> ld_input_files;
for (TempFile* obj_file : *obj_files_) {
ld_input_files.push_back(
GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_READ));
GetHandleForSubprocess(obj_file, PP_FILEOPENFLAG_READ,
ld_channel_peer_pid_));
}

int64_t link_start_time = NaClGetTimeOfDayMicroseconds();
Expand Down
17 changes: 10 additions & 7 deletions components/nacl/renderer/plugin/pnacl_translate_thread.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ class PnaclTranslateThread {
bool started() const { return coordinator_ != NULL; }

private:
ppapi::proxy::SerializedHandle GetHandleForSubprocess(TempFile* file,
int32_t open_flags);
ppapi::proxy::SerializedHandle GetHandleForSubprocess(
TempFile* file, int32_t open_flags, base::ProcessId peer_pid);

// Helper thread entry point for compilation. Takes a pointer to
// PnaclTranslateThread and calls DoCompile().
Expand Down Expand Up @@ -137,7 +137,7 @@ class PnaclTranslateThread {
struct NaClMutex cond_mu_;
// Data buffers from FileDownloader are enqueued here to pass from the
// main thread to the SRPC thread. Protected by cond_mu_
std::deque<std::vector<char> > data_buffers_;
std::deque<std::string> data_buffers_;
// Whether all data has been downloaded and copied to translation thread.
// Associated with buffer_cond_
bool done_;
Expand All @@ -154,12 +154,15 @@ class PnaclTranslateThread {
std::string architecture_attributes_;
PnaclCoordinator* coordinator_;

// This IPC::SyncChannel can only be used and freed by the parent thread.
// These IPC::SyncChannels can only be used and freed by the parent thread.
scoped_ptr<IPC::SyncChannel> compiler_channel_;
scoped_ptr<IPC::SyncChannel> ld_channel_;
// This IPC::SyncMessageFilter can be used by the child thread.
// These IPC::SyncMessageFilters can be used by the child thread.
scoped_refptr<IPC::SyncMessageFilter> compiler_channel_filter_;
scoped_refptr<IPC::SyncMessageFilter> ld_channel_filter_;
// PID of the subprocess, needed for copying handles to the subprocess on
// Windows. This is used by the child thread.
// PIDs of the subprocesses, needed for copying handles to the subprocess
// on Windows. These are used by the child thread.
base::ProcessId compiler_channel_peer_pid_;
base::ProcessId ld_channel_peer_pid_;

private:
Expand Down
Loading

0 comments on commit 98d206c

Please sign in to comment.