Commit fe8ed20c authored by Bruce Dawson's avatar Bruce Dawson Committed by Commit Bot

Get installer to do smaller disk writes

Writes of large blocks of memory can cause kernel address-space pressure
which can be fatal on 32-bit Windows. Yes, we still have some customers
using 32-bit Windows.

This change updates PEResource::WriteToDisk to write a maximum of 8 MiB
at a time. This should not affect performance and will avoid failures.

This change also updates the mini_installer copy of this file to use
nullptr for consistency.

Bug: 1001022
Change-Id: Icca3122a7decc7ce6ce89cc742c988d68737ff28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1793531
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: default avatarSorin Jianu <sorin@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695755}
parent 81cffc0e
...@@ -4,17 +4,19 @@ ...@@ -4,17 +4,19 @@
#include "chrome/installer/mini_installer/pe_resource.h" #include "chrome/installer/mini_installer/pe_resource.h"
#include <algorithm>
PEResource::PEResource(HRSRC resource, HMODULE module) PEResource::PEResource(HRSRC resource, HMODULE module)
: resource_(resource), module_(module) { : resource_(resource), module_(module) {
} }
PEResource::PEResource(const wchar_t* name, const wchar_t* type, HMODULE module) PEResource::PEResource(const wchar_t* name, const wchar_t* type, HMODULE module)
: resource_(NULL), module_(module) { : resource_(nullptr), module_(module) {
resource_ = ::FindResource(module, name, type); resource_ = ::FindResource(module, name, type);
} }
bool PEResource::IsValid() { bool PEResource::IsValid() {
return (NULL != resource_); return nullptr != resource_;
} }
size_t PEResource::Size() { size_t PEResource::Size() {
...@@ -23,26 +25,35 @@ size_t PEResource::Size() { ...@@ -23,26 +25,35 @@ size_t PEResource::Size() {
bool PEResource::WriteToDisk(const wchar_t* full_path) { bool PEResource::WriteToDisk(const wchar_t* full_path) {
// Resource handles are not real HGLOBALs so do not attempt to close them. // Resource handles are not real HGLOBALs so do not attempt to close them.
// Windows frees them whenever there is memory pressure. // Resources are freed when the containing module is unloaded.
HGLOBAL data_handle = ::LoadResource(module_, resource_); HGLOBAL data_handle = ::LoadResource(module_, resource_);
if (NULL == data_handle) { if (nullptr == data_handle)
return false; return false;
}
void* data = ::LockResource(data_handle); const char* data = reinterpret_cast<const char*>(::LockResource(data_handle));
if (NULL == data) { if (nullptr == data)
return false; return false;
}
size_t resource_size = Size(); const size_t resource_size = Size();
HANDLE out_file = ::CreateFile(full_path, GENERIC_WRITE, 0, NULL, HANDLE out_file = ::CreateFile(full_path, GENERIC_WRITE, 0, nullptr,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
if (INVALID_HANDLE_VALUE == out_file) { if (INVALID_HANDLE_VALUE == out_file)
return false; return false;
}
DWORD written = 0; // Don't write all of the data at once because this can lead to kernel
if (!::WriteFile(out_file, data, static_cast<DWORD>(resource_size), // address-space exhaustion on 32-bit Windows (see https://crbug.com/1001022
&written, NULL)) { // for details).
constexpr size_t kMaxWriteAmount = 8 * 1024 * 1024;
for (size_t total_written = 0; total_written < resource_size; /**/) {
const size_t write_amount =
std::min(kMaxWriteAmount, resource_size - total_written);
DWORD written = 0;
if (!::WriteFile(out_file, data + total_written,
static_cast<DWORD>(write_amount), &written, nullptr)) {
::CloseHandle(out_file); ::CloseHandle(out_file);
return false; return false;
}
total_written += write_amount;
} }
return ::CloseHandle(out_file) ? true : false; return ::CloseHandle(out_file) ? true : false;
} }
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/updater/win/installer/pe_resource.h" #include "chrome/updater/win/installer/pe_resource.h"
#include <algorithm>
namespace updater { namespace updater {
PEResource::PEResource(const wchar_t* name, const wchar_t* type, HMODULE module) PEResource::PEResource(const wchar_t* name, const wchar_t* type, HMODULE module)
...@@ -21,26 +23,35 @@ size_t PEResource::Size() { ...@@ -21,26 +23,35 @@ size_t PEResource::Size() {
bool PEResource::WriteToDisk(const wchar_t* full_path) { bool PEResource::WriteToDisk(const wchar_t* full_path) {
// Resource handles are not real HGLOBALs so do not attempt to close them. // Resource handles are not real HGLOBALs so do not attempt to close them.
// Windows frees them whenever there is memory pressure. // Resources are freed when the containing module is unloaded.
HGLOBAL data_handle = ::LoadResource(module_, resource_); HGLOBAL data_handle = ::LoadResource(module_, resource_);
if (nullptr == data_handle) if (nullptr == data_handle)
return false; return false;
void* data = ::LockResource(data_handle); const char* data = reinterpret_cast<const char*>(::LockResource(data_handle));
if (nullptr == data) if (nullptr == data)
return false; return false;
size_t resource_size = Size(); const size_t resource_size = Size();
HANDLE out_file = ::CreateFile(full_path, GENERIC_WRITE, 0, nullptr, HANDLE out_file = ::CreateFile(full_path, GENERIC_WRITE, 0, nullptr,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
if (INVALID_HANDLE_VALUE == out_file) if (INVALID_HANDLE_VALUE == out_file)
return false; return false;
DWORD written = 0; // Don't write all of the data at once because this can lead to kernel
if (!::WriteFile(out_file, data, static_cast<DWORD>(resource_size), &written, // address-space exhaustion on 32-bit Windows (see https://crbug.com/1001022
nullptr)) { // for details).
::CloseHandle(out_file); constexpr size_t kMaxWriteAmount = 8 * 1024 * 1024;
return false; for (size_t total_written = 0; total_written < resource_size; /**/) {
const size_t write_amount =
std::min(kMaxWriteAmount, resource_size - total_written);
DWORD written = 0;
if (!::WriteFile(out_file, data + total_written,
static_cast<DWORD>(write_amount), &written, nullptr)) {
::CloseHandle(out_file);
return false;
}
total_written += write_amount;
} }
return ::CloseHandle(out_file) ? true : false; return ::CloseHandle(out_file) ? true : false;
} }
......
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