Commit 5c4ad57d authored by mattm@chromium.org's avatar mattm@chromium.org

posix LaunchProcess: Don't use STL iterators after fork.

Fixes some deadlocks on linux debug caused by STL debug iterators using locks.

BUG=331459

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243401 0039d316-1c4b-4281-b951-d872f2087c98
parent e25a1787
...@@ -19,20 +19,24 @@ bool PerformInjectiveMultimapDestructive( ...@@ -19,20 +19,24 @@ bool PerformInjectiveMultimapDestructive(
int extra_fds[kMaxExtraFDs]; int extra_fds[kMaxExtraFDs];
unsigned next_extra_fd = 0; unsigned next_extra_fd = 0;
// DANGER: this function may not allocate. // DANGER: this function must not allocate or lock.
// Cannot use STL iterators here, since debug iterators use locks.
for (InjectiveMultimap::iterator i = m->begin(); i != m->end(); ++i) { for (size_t i_index = 0; i_index < m->size(); ++i_index) {
InjectiveMultimap::value_type* i = &(*m)[i_index];
int temp_fd = -1; int temp_fd = -1;
// We DCHECK the injectiveness of the mapping. // We DCHECK the injectiveness of the mapping.
for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) { for (size_t j_index = i_index + 1; j_index < m->size(); ++j_index) {
InjectiveMultimap::value_type* j = &(*m)[j_index];
DCHECK(i->dest != j->dest) << "Both fd " << i->source DCHECK(i->dest != j->dest) << "Both fd " << i->source
<< " and " << j->source << " map to " << i->dest; << " and " << j->source << " map to " << i->dest;
} }
const bool is_identity = i->source == i->dest; const bool is_identity = i->source == i->dest;
for (InjectiveMultimap::iterator j = i + 1; j != m->end(); ++j) { for (size_t j_index = i_index + 1; j_index < m->size(); ++j_index) {
InjectiveMultimap::value_type* j = &(*m)[j_index];
if (!is_identity && i->dest == j->source) { if (!is_identity && i->dest == j->source) {
if (temp_fd == -1) { if (temp_fd == -1) {
if (!delegate->Duplicate(&temp_fd, i->dest)) if (!delegate->Duplicate(&temp_fd, i->dest))
......
...@@ -207,7 +207,7 @@ static const char kFDDir[] = "/proc/self/fd"; ...@@ -207,7 +207,7 @@ static const char kFDDir[] = "/proc/self/fd";
#endif #endif
void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) { void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
// DANGER: no calls to malloc are allowed from now on: // DANGER: no calls to malloc or locks are allowed from now on:
// http://crbug.com/36678 // http://crbug.com/36678
// Get the maximum number of FDs possible. // Get the maximum number of FDs possible.
...@@ -220,12 +220,13 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) { ...@@ -220,12 +220,13 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
const int fd = static_cast<int>(i); const int fd = static_cast<int>(i);
if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO) if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
continue; continue;
InjectiveMultimap::const_iterator j; // Cannot use STL iterators here, since debug iterators use locks.
for (j = saved_mapping.begin(); j != saved_mapping.end(); j++) { size_t j;
if (fd == j->dest) for (j = 0; j < saved_mapping.size(); j++) {
if (fd == saved_mapping[j].dest)
break; break;
} }
if (j != saved_mapping.end()) if (j < saved_mapping.size())
continue; continue;
// Since we're just trying to close anything we can find, // Since we're just trying to close anything we can find,
...@@ -249,12 +250,13 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) { ...@@ -249,12 +250,13 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
continue; continue;
if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO) if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
continue; continue;
InjectiveMultimap::const_iterator i; // Cannot use STL iterators here, since debug iterators use locks.
for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) { size_t i;
if (fd == i->dest) for (i = 0; i < saved_mapping.size(); i++) {
if (fd == saved_mapping[i].dest)
break; break;
} }
if (i != saved_mapping.end()) if (i < saved_mapping.size())
continue; continue;
if (fd == dir_fd) if (fd == dir_fd)
continue; continue;
...@@ -388,7 +390,7 @@ bool LaunchProcess(const std::vector<std::string>& argv, ...@@ -388,7 +390,7 @@ bool LaunchProcess(const std::vector<std::string>& argv,
memset(reinterpret_cast<void*>(malloc), 0xff, 8); memset(reinterpret_cast<void*>(malloc), 0xff, 8);
#endif // 0 #endif // 0
// DANGER: no calls to malloc are allowed from now on: // DANGER: no calls to malloc or locks are allowed from now on:
// http://crbug.com/36678 // http://crbug.com/36678
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
...@@ -406,11 +408,12 @@ bool LaunchProcess(const std::vector<std::string>& argv, ...@@ -406,11 +408,12 @@ bool LaunchProcess(const std::vector<std::string>& argv,
#endif // defined(OS_CHROMEOS) #endif // defined(OS_CHROMEOS)
if (options.fds_to_remap) { if (options.fds_to_remap) {
for (FileHandleMappingVector::const_iterator // Cannot use STL iterators here, since debug iterators use locks.
it = options.fds_to_remap->begin(); for (size_t i = 0; i < options.fds_to_remap->size(); ++i) {
it != options.fds_to_remap->end(); ++it) { const FileHandleMappingVector::value_type& value =
fd_shuffle1.push_back(InjectionArc(it->first, it->second, false)); (*options.fds_to_remap)[i];
fd_shuffle2.push_back(InjectionArc(it->first, it->second, false)); fd_shuffle1.push_back(InjectionArc(value.first, value.second, false));
fd_shuffle2.push_back(InjectionArc(value.first, value.second, false));
} }
} }
...@@ -521,7 +524,7 @@ static GetAppOutputInternalResult GetAppOutputInternal( ...@@ -521,7 +524,7 @@ static GetAppOutputInternalResult GetAppOutputInternal(
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
RestoreDefaultExceptionHandler(); RestoreDefaultExceptionHandler();
#endif #endif
// DANGER: no calls to malloc are allowed from now on: // DANGER: no calls to malloc or locks are allowed from now on:
// http://crbug.com/36678 // http://crbug.com/36678
// Obscure fork() rule: in the child, if you don't end up doing exec*(), // Obscure fork() rule: in the child, if you don't end up doing exec*(),
......
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