OOP HP: Create a 10-s timeout for writing to the memlog pipe. |
||
Issue descriptionIf there's a problem with the profiling process and clearing the pipe, we should cancel the write to the pipe and stop profiling.
,
Oct 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4b32fd122bdba3e754d5f7b5264e9199c350432 commit c4b32fd122bdba3e754d5f7b5264e9199c350432 Author: Philip Rogers <pdr@chromium.org> Date: Wed Oct 25 23:54:12 2017 Revert "OOP HP: Add 10 second timeout." This reverts commit e1d7f2223ede76d6d7db6eb1df940abb6f9d4dc4. Reason for revert: Broke build: https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux%20x64/builds/23153 Original change's description: > OOP HP: Add 10 second timeout. > > This CL changes the MemlogSenderPipe to use a synchronous, non-blocking write > with a 10-second timeout rather than a synchronous, blocking write. The latter > could cause a process to stop functioning correctly if the pipe is not cleared > in a timely fashion. The new mechanism uses poll() on POSIX and SleepEx() on > Windows to wait synchronously with a timeout. After the timeout, the allocator > shim is shut down. > > This requires the OS pipe to have slightly different characteristics than > allowed by PlatformChannelPair. On Windows, we want the pipe to have a 64k > buffer rather than a 4k buffer. For performance reasons, on macOS and Linux, we > want to use a pipe rather than a socket. This CL changes the AllocatorShim to > create its own pipes, rather than using mojo::edk::PlatformChannelPair. > > This CL updates the tear down logic in memlog_allocator_shim.cc to be thread > safe, and to close its end of the pipe on error. > > Bug: 777546 > Change-Id: Ieeacfdac990178c0e0d04295bb33d555675c8b47 > Reviewed-on: https://chromium-review.googlesource.com/731473 > Commit-Queue: Erik Chen <erikchen@chromium.org> > Reviewed-by: Albert J. Wong <ajwong@chromium.org> > Reviewed-by: Robert Liao <robliao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#511635} TBR=ajwong@chromium.org,brettw@chromium.org,robliao@chromium.org,erikchen@chromium.org Change-Id: Iec34fa79b665052925982ec80e7e78289d069682 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 777546 Reviewed-on: https://chromium-review.googlesource.com/738619 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#511649} [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/browser/profiling_host/profiling_process_host.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/BUILD.gn [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_allocator_shim.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_sender_pipe.h [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_sender_pipe_posix.cc [add] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_sender_pipe_posix.h [delete] https://crrev.com/8bd4d807a9b437cfdbc767d813d11407eb30c2da/chrome/common/profiling/memlog_sender_pipe_unittest.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_sender_pipe_win.cc [add] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/memlog_sender_pipe_win.h [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/common/profiling/profiling_client.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/profiling/memlog_receiver_pipe_posix.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/profiling/memlog_receiver_pipe_win.cc [modify] https://crrev.com/c4b32fd122bdba3e754d5f7b5264e9199c350432/chrome/test/BUILD.gn
,
Oct 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01aa222c6880da9d8e93dac6af4891b470c0a0be commit 01aa222c6880da9d8e93dac6af4891b470c0a0be Author: erikchen <erikchen@chromium.org> Date: Thu Oct 26 02:43:09 2017 [Reland #1] OOP HP: Add 10 second timeout. The original CL didn't handle the return value of pipe(), which caused a compile error on the official Linux builder. Original change's description: > OOP HP: Add 10 second timeout. > > This CL changes the MemlogSenderPipe to use a synchronous, non-blocking write > with a 10-second timeout rather than a synchronous, blocking write. The latter > could cause a process to stop functioning correctly if the pipe is not cleared > in a timely fashion. The new mechanism uses poll() on POSIX and SleepEx() on > Windows to wait synchronously with a timeout. After the timeout, the allocator > shim is shut down. > > This requires the OS pipe to have slightly different characteristics than > allowed by PlatformChannelPair. On Windows, we want the pipe to have a 64k > buffer rather than a 4k buffer. For performance reasons, on macOS and Linux, we > want to use a pipe rather than a socket. This CL changes the AllocatorShim to > create its own pipes, rather than using mojo::edk::PlatformChannelPair. > > This CL updates the tear down logic in memlog_allocator_shim.cc to be thread > safe, and to close its end of the pipe on error. > > Bug: 777546 > Change-Id: Ieeacfdac990178c0e0d04295bb33d555675c8b47 > Reviewed-on: https://chromium-review.googlesource.com/731473 > Commit-Queue: Erik Chen <erikchen@chromium.org> > Reviewed-by: Albert J. Wong <ajwong@chromium.org> > Reviewed-by: Robert Liao <robliao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#511635} Bug: 777546 Change-Id: I2c685e95a12b1451d2274124962ebc8b70a160c3 TBR: ajwong@chromium.org Reviewed-on: https://chromium-review.googlesource.com/738410 Reviewed-by: Erik Chen <erikchen@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#511711} [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/browser/profiling_host/profiling_process_host.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/BUILD.gn [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/memlog_allocator_shim.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/memlog_sender_pipe.h [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/memlog_sender_pipe_posix.cc [delete] https://crrev.com/9eaf25f5ca709182b2be85a407ac3727ee16c7d4/chrome/common/profiling/memlog_sender_pipe_posix.h [add] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/memlog_sender_pipe_unittest.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/memlog_sender_pipe_win.cc [delete] https://crrev.com/9eaf25f5ca709182b2be85a407ac3727ee16c7d4/chrome/common/profiling/memlog_sender_pipe_win.h [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/common/profiling/profiling_client.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/profiling/memlog_receiver_pipe_posix.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/profiling/memlog_receiver_pipe_win.cc [modify] https://crrev.com/01aa222c6880da9d8e93dac6af4891b470c0a0be/chrome/test/BUILD.gn
,
Nov 17 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Oct 25 2017