Issue metadata
Sign in to add a comment
|
MemoryDumpManager::OnTraceLogDisabled need not be called on FILE thread |
||||||||||||||||||||||||
Issue description- go to about://tracing and hit record. - select memory-infra and hit record. - open some tabs and wait until the buffer is full or almost full (do not stop recording). - chrome crashes with the following: [11098:11098:0316/132502.228493:FATAL:thread_restrictions.cc(38)] Check failed: false. Function marked as IO-only was called from a thread that disallows IO! If this thread really should be allowed to make IO calls, adjust the call to base::ThreadRestrictions::SetIOAllowed() in this thread's startup. #0 0x7f9da00e8af7 base::debug::StackTrace::StackTrace() #1 0x7f9da010ce1b logging::LogMessage::~LogMessage() #2 0x7f9da018f645 base::ThreadRestrictions::AssertIOAllowed() #3 0x7f9da0183eba base::PlatformThread::Join() #4 0x7f9da018b690 base::Thread::Stop() #5 0x7f9da01a059b base::trace_event::MemoryDumpManager::OnTraceLogDisabled() #6 0x7f9da01ba5fa base::trace_event::TraceLog::SetDisabledWhileLocked() #7 0x7f9da01b6dba base::trace_event::TraceLog::ThreadLocalEventBuffer::AddTraceEvent() #8 0x7f9da01bcb67 base::trace_event::TraceLog::AddTraceEventWithThreadIdAndTimestamp() #9 0x7f9da00e970a base::debug::TaskAnnotator::RunTask() #10 0x7f9da011a2dd base::MessageLoop::RunTask() #11 0x7f9da011ab4e base::MessageLoop::DoDelayedWork() #12 0x7f9da011cc10 base::(anonymous namespace)::WorkSourceDispatch() #13 0x7f9d9910ce04 g_main_context_dispatch #14 0x7f9d9910d048 <unknown> #15 0x7f9d9910d0ec g_main_context_iteration #16 0x7f9da011c946 base::MessagePumpGlib::Run() #17 0x7f9da011a02e base::MessageLoop::RunHandler() #18 0x7f9da014e91c base::RunLoop::Run() #19 0x7f9da13385fa ChromeBrowserMainParts::MainMessageLoopRun() #20 0x7f9d9d7f5989 content::BrowserMainLoop::RunMainMessageLoopParts() #21 0x7f9d9d7f9917 content::BrowserMainRunnerImpl::Run() #22 0x7f9d9d7f0b4e content::BrowserMain() #23 0x7f9d9e01ad83 content::RunNamedProcessTypeMain() #24 0x7f9d9e01ba96 content::ContentMainRunnerImpl::Run() #25 0x7f9d9e01a120 content::ContentMain() #26 0x7f9da0c1a291 ChromeMain #27 0x7f9d96707f45 __libc_start_main #28 0x7f9da0c1a129 <unknown>
,
Mar 16 2017
This is tricky one. Should we really disable tracing when trace buffer is full? Can we just disable category flags? If we should disable, then is it fine to do a Thread::Stop on any thread in that case? If it's fine then we could just add ScopedIOAllowed. If it's not fine, then it's harder to solve this issue. We need to store the task runner of FILE thread somehow in MDM and post task. But, in some cases we do not get enabled call from FILE thread. So, it's really difficult to get the FILE thread in MDM. Maybe this can be done after moving to mojo. Personally I think we should just fix this issue with IOAlloed. I am not sure how thread stops are handled in base where we don't have concept of FILE thread.
,
Mar 16 2017
Also, it's very rare this case happens and the trace can be loaded. On top of that, leaving memory-infra dumps to fill the buffer would never really load in the tracing UI because of trace size.
,
Mar 16 2017
> Should we really disable tracing when trace buffer is full?
I think this is already what happens in "record until full" mode no?
> Can we just disable category flags?
I wouldn't change that behavior.
Yeah so quite sucks that we essentially can end up disabling the trace on a random thread. But it's quite hard to change now, and given all the things on flight don't feel to comfortable having to even think to that change right now.
I wonder if this can all be solved with
base::TaskScheduler::CreateSequencedTaskRunnerWithTraits(MayBlock)
Aternatively, if OnTraceLogEnabled happens in the right (file) thread we might just cache the task runner, and OnTraceLogDisabled we could do:
if(!cached_taskrunner_.RunsOnCurrentThread()) {
cached_task_runner->PostTask(OnTraceLogDisabled, ..)
return;
}
BTW given the upcoming refactorings I probably woudln't bother to do this now, unless it's a few lines change
,
Mar 16 2017
The second option is not possible. The OnTraceLogEnabled is also not called on FILE thread always. In startup tracing the enabled is called before the browser threads are created. So, it's impossible to get the file thread. I will look into the Sequenced task runner. Yes not a priority, just noting down my comments for future.
,
Mar 21 2017
I'm running protractor tests integrated with benchpress, using this as an example (https://github.com/angular/benchpress-tree), and when running such tests on Chrome 57 I started facing the following issue: Failed: Error: The DevTools trace buffer filled during the test! May this be related to this issue? Note: This was not happening on Chrome v56.
,
Mar 21 2017
Re #6: don't think this is related. This issue is hit only in builds with DCHECK enabled, not in production chrome.
,
Mar 21 2017
Re #6: I think the issue you are facing could be because of extra trace events added in Chrome from M56 - M57. Try disabling not useful categories and you'd not see the message.
,
Mar 22 2017
Unfortunately, even if I use only this category devtools.timeline the issue still happens
,
Mar 22 2017
Please don't jam the bug. Move the discussion on another bug if interested. #7-#9 are about a completely different problem.
,
Apr 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7af20094a387dfa6a3f0eb5fb5b15066bdf90f19 commit 7af20094a387dfa6a3f0eb5fb5b15066bdf90f19 Author: hjd <hjd@chromium.org> Date: Thu Apr 27 14:40:18 2017 memory-infra: Never kill memory-infra background thread We want ultimately to move to a model where memory-infra does not have its own custom thread and instead uses base::scheduler. However this is risky at the moment given the tracing entanglement and the various other refactorings going on. For now we change the logic to never kill the background thread. This will simplify the logic of the MemoryDumpManager significantly and pave the way for us to disentangle tracing. BUG= 705564 BUG= 702289 Review-Url: https://codereview.chromium.org/2836933002 Cr-Commit-Position: refs/heads/master@{#467671} [modify] https://crrev.com/7af20094a387dfa6a3f0eb5fb5b15066bdf90f19/base/trace_event/memory_dump_manager.cc [modify] https://crrev.com/7af20094a387dfa6a3f0eb5fb5b15066bdf90f19/base/trace_event/memory_dump_manager.h
,
Sep 12 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by chiniforooshan@chromium.org
, Mar 16 2017