Commit ae9c454b authored by ananta's avatar ananta Committed by Commit bot

Attempt to fix a CloseHandle crasher in the renderer process. The crash is triggered by Nacl.

Based on the crash dump, the crash occurs while loading a Nacl module in the renderer process. The Nacl translate
thread has a valid file handle which is created by the Nacl host in the browser. It then calls into the Nacl loader
to load the module which fails. The Nacl loading code in LaunchSelLdr function is closing the file handle which is
passed in. Based on comments in the PnaclTranslateThread class, ownership of the file handle is only transferred on success.

Thus when the call returns the PnaclTranslateThread code tries to close the file handle which is already closed. In the
meantime the Windows handle is reused to something else which is tracked by our handle tracker. The second CloseHandle
attempt causes a CHECK to fire because we are closing a handle which is being tracked.

Fix is to not close the file handle in the PnaclTranslateThread class as ownership is transferred on
call to LaunchSelHdr.

BUG=426582, 475872

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

Cr-Commit-Position: refs/heads/master@{#329251}
parent c55c0a6c
...@@ -80,6 +80,8 @@ class Plugin : public pp::Instance { ...@@ -80,6 +80,8 @@ class Plugin : public pp::Instance {
// should include a time-out at which point we declare the // should include a time-out at which point we declare the
// nacl_ready_state to be done, and let the normal crash detection // nacl_ready_state to be done, and let the normal crash detection
// mechanism(s) take over. // mechanism(s) take over.
// Please note that a call to this function takes over ownership of the
// file_info pointer.
void LoadNaClModule(PP_NaClFileInfo file_info, void LoadNaClModule(PP_NaClFileInfo file_info,
bool uses_nonsfi_mode, bool uses_nonsfi_mode,
PP_NaClAppProcessType process_type); PP_NaClAppProcessType process_type);
...@@ -89,6 +91,8 @@ class Plugin : public pp::Instance { ...@@ -89,6 +91,8 @@ class Plugin : public pp::Instance {
// Blocks until the helper module signals initialization is done. // Blocks until the helper module signals initialization is done.
// Does not update nacl_module_origin(). // Does not update nacl_module_origin().
// Returns NULL or the NaClSubprocess of the new helper NaCl module. // Returns NULL or the NaClSubprocess of the new helper NaCl module.
// Please note that a call to this function takes over ownership of the
// file_info pointer.
NaClSubprocess* LoadHelperNaClModule(const std::string& helper_url, NaClSubprocess* LoadHelperNaClModule(const std::string& helper_url,
PP_NaClFileInfo file_info, PP_NaClFileInfo file_info,
ErrorInfo* error_info); ErrorInfo* error_info);
......
...@@ -178,14 +178,12 @@ void PnaclTranslateThread::DoTranslate() { ...@@ -178,14 +178,12 @@ void PnaclTranslateThread::DoTranslate() {
PnaclResources::ResourceType compiler_type = pnacl_options_->use_subzero PnaclResources::ResourceType compiler_type = pnacl_options_->use_subzero
? PnaclResources::SUBZERO ? PnaclResources::SUBZERO
: PnaclResources::LLC; : PnaclResources::LLC;
// On success, ownership of file_info is transferred. // Ownership of file_info is transferred here.
PP_NaClFileInfo file_info = resources_->TakeFileInfo(compiler_type); PP_NaClFileInfo file_info = resources_->TakeFileInfo(compiler_type);
const std::string& url = resources_->GetUrl(compiler_type); const std::string& url = resources_->GetUrl(compiler_type);
NaClSubprocess* compiler_subprocess = NaClSubprocess* compiler_subprocess =
plugin_->LoadHelperNaClModule(url, file_info, &error_info); plugin_->LoadHelperNaClModule(url, file_info, &error_info);
if (compiler_subprocess == NULL) { if (compiler_subprocess == NULL) {
if (file_info.handle != PP_kInvalidFileHandle)
CloseFileHandle(file_info.handle);
TranslateFailed(PP_NACL_ERROR_PNACL_LLC_SETUP, TranslateFailed(PP_NACL_ERROR_PNACL_LLC_SETUP,
"Compile process could not be created: " + "Compile process could not be created: " +
error_info.message()); error_info.message());
...@@ -352,12 +350,10 @@ bool PnaclTranslateThread::RunLdSubprocess() { ...@@ -352,12 +350,10 @@ bool PnaclTranslateThread::RunLdSubprocess() {
nacl::DescWrapper* ld_out_file = nexe_file_->write_wrapper(); nacl::DescWrapper* ld_out_file = nexe_file_->write_wrapper();
int64_t ld_start_time = NaClGetTimeOfDayMicroseconds(); int64_t ld_start_time = NaClGetTimeOfDayMicroseconds();
PP_NaClFileInfo ld_file_info = resources_->TakeFileInfo(PnaclResources::LD); PP_NaClFileInfo ld_file_info = resources_->TakeFileInfo(PnaclResources::LD);
// On success, ownership of ld_file_info is transferred. // Ownership of ld_file_info is transferred here.
nacl::scoped_ptr<NaClSubprocess> ld_subprocess(plugin_->LoadHelperNaClModule( nacl::scoped_ptr<NaClSubprocess> ld_subprocess(plugin_->LoadHelperNaClModule(
resources_->GetUrl(PnaclResources::LD), ld_file_info, &error_info)); resources_->GetUrl(PnaclResources::LD), ld_file_info, &error_info));
if (ld_subprocess.get() == NULL) { if (ld_subprocess.get() == NULL) {
if (ld_file_info.handle != PP_kInvalidFileHandle)
CloseFileHandle(ld_file_info.handle);
TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP, TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP,
"Link process could not be created: " + "Link process could not be created: " +
error_info.message()); error_info.message());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment