New issue
Advanced search Search tips

Issue 904743 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

ProcessBacktrace in stack_trace_posix.cc is not thread-safe

Project Member Reported by hajimehoshi@chromium.org, Nov 13

Issue description

This uses google::Symbolize, but this is not thread-safe since it manipulats a proc file without locks.
 
https://github.com/google/glog/pull/388 is an upstream PR to fix this.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df46b528df23b652ae20473c318ed0568d66172a

commit df46b528df23b652ae20473c318ed0568d66172a
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed Nov 14 08:25:53 2018

Make ProcessBacktrace thread-safe

google::Symbolize is not thread-safe since it manipulats a proc file
without locks.

This CL makes ProcessBacktrace thread-safe by base::Lock.

Bug:  904743 

Change-Id: I12eb0f27158f39baa9683d74c98f4defdf0449f1
Reviewed-on: https://chromium-review.googlesource.com/c/1329814
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607937}
[modify] https://crrev.com/df46b528df23b652ae20473c318ed0568d66172a/base/debug/stack_trace_posix.cc

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f2959908f72e8b010a21c978dfa513a31d7fbc6

commit 3f2959908f72e8b010a21c978dfa513a31d7fbc6
Author: tzik <tzik@chromium.org>
Date: Tue Dec 04 20:32:23 2018

Uprev //base/third_party/symbolize

This CL update //base/third_party/symbolize to the latest version by
copying upstream symbolize.{h,cc} and demangle.{h,cc} as-is, so
that google::Symbolize() gets thread-safe.

//sandbox change is needed to allow pread() used by symbolize.cc.
This should be safe since we already allow lseek() and read().

Bug:  904743 
Change-Id: I50b8389dfa18175e8c734c26440f651f74e6daa4
Reviewed-on: https://chromium-review.googlesource.com/c/1334991
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613679}
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/PRESUBMIT.py
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/README.chromium
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/config.h
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/demangle.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/demangle.h
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/symbolize.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/base/third_party/symbolize/symbolize.h
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/components/nacl/loader/sandbox_linux/nacl_bpf_sandbox_linux.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/services/service_manager/sandbox/linux/bpf_cdm_policy_linux.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/services/service_manager/sandbox/linux/bpf_pdf_compositor_policy_linux.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/services/service_manager/sandbox/linux/bpf_ppapi_policy_linux.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/services/service_manager/sandbox/linux/bpf_renderer_policy_linux.cc
[modify] https://crrev.com/3f2959908f72e8b010a21c978dfa513a31d7fbc6/services/service_manager/sandbox/linux/bpf_utility_policy_linux.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e73d36521e120960c2980acca45a55d7568495e6

commit e73d36521e120960c2980acca45a55d7568495e6
Author: tzik <tzik@chromium.org>
Date: Wed Dec 12 14:52:38 2018

Remove a no longer needed lock around google::Symbolize

This reverts https://crrev.com/df46b528df23b652. After uprev'ing
//base/third_party/symbolize at https://crrev.com/3f2959908f72e8b0,
google::Symbolize is thread-safe itself, and the lock around here is
no longer needed.

Bug:  904743 
Change-Id: If6d659491ebcca548363a98c91334693334f6a0b
Reviewed-on: https://chromium-review.googlesource.com/c/1362712
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615885}
[modify] https://crrev.com/e73d36521e120960c2980acca45a55d7568495e6/base/debug/stack_trace_posix.cc

Sign in to add a comment