New issue
Advanced search Search tips

Issue 716172 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 715656



Sign in to add a comment

Browser process crashes before log-net-log is fully written.

Project Member Reported by ta...@google.com, Apr 27 2017

Issue description

Hello!

From #715656, we've noticed two issues:

1. the log-net-log file doesn't contain a valid JSON.
2. our HTML file requires 3 JS files... but one of them is not present in log-net-log.

We think it is because the browser crashes before it can call WriteToFileNetLogObserver::StopObserving (https://cs.chromium.org/chromium/src/net/log/write_to_file_net_log_observer.cc?type=cs&l=70)


For number 2, we think it's because we don't flush at the end of WriteToFileNetLogObserver::OnAddEntry.

Two questions:
- It seems we should reformat the JSON not to be a giant JSON. Each line should be its own valid JSON. That'd solve the invalid JSON issue.
- Can we flush at the end of WriteToFileNetLogObserver::OnAddEntry? (If reformatting JSON is not good, we would like this one.)

Thank you
 

Comment 1 by eroman@chromium.org, Apr 28 2017

Can you explain more what the context of this is?

What are you using these net log files as a dependency of?

Comment 2 by ta...@google.com, Apr 28 2017

> Can you explain more what the context of this is?
> What are you using these net log files as a dependency of?

ClusterFuzz (fuzzing chrome) depends on the net log file's content to identify which files are needed.

For example, when we run chrome on testcase.html, we see that testcase.html loads testharness.js. We'll see this in the net log file and will save testharness.js along with testcase.html.

So, we'd like the net log file to be accurate even when a browser crash.

Comment 3 by aarya@google.com, Apr 28 2017

Basically --log-net-log is our platform independent way of knowing which resources of a testcase are loaded at runtime and then we archive those in a zip. that way, we don't need developers to place stuff in certain dirs in layouttests/etc and many times dev can't do that (e.g. webkit tests, etc).

Comment 4 by ta...@google.com, Apr 28 2017

Blocking: 715656

Comment 5 by eroman@chromium.org, Apr 28 2017

Labels: -Pri-1 Pri-2
> - Can we flush at the end of WriteToFileNetLogObserver::OnAddEntry?

No, not really.

The class you have highlighted runs file I/O synchronously on the calling thread (which is usually the IO thread). Flushing here would make things worse.

We have a replacement implementation, FileNetLogObserver, which is what the code for --log-net-log will be moving to. FileNetLogObserver offloads the file writing to a separate thread, so we could certainly flush here after events are written. However, this implementation does some buffering of events, and involves some thread signalling too...

So depending how soon after initiating the request your crash occurs, this may not be sufficient for always tracing the dependencies.

As for the JSON malformedness, I would consider it a non-goal to re-write the file to preserve well-formedness while streaming events.

So all-in-all, what I can offer you is flushing via FileNetLogObserver, with the caveats mentioned above.

If that is insufficient, --log-net-log probably isn't a good fit for doing this dependency tracing.

Instead you may want a simpler feature that solely traces the URLs that are visited to. This could use a simple file format (i.e. just a URL list), could write immediately to disk from the calling thread, and could probably use a memory-mapped file for better crash resistance.

Comment 6 by ta...@google.com, May 1 2017

> Instead you may want a simpler feature that solely traces the URLs that are visited to. This could use a simple file format (i.e. just a URL list), could write immediately to disk from the calling thread, and could probably use a memory-mapped file for better crash resistance.

I think we would like this.

I'm not sure I understand it completely. Could you elaborate more on how to achieve this?
Re comment #6: A separate mechanism used exclusively for tracing URLs could look as follows:

(A) Using LOG(INFO) or VLOG(???). I don't know how resilient our standard logging mechanism is to crashes, but if it is good enough you could probably just throw in a single LOG() statement somewhere in the code (if there isn't already) and log the start of URL requests. Seems generic enough to justify having as a log statement. You would need to also enable logging (https://www.chromium.org/for-testers/enable-logging) when launching chrome from your test harness.

or

(B) Building a separate feature gated behind a command-line flag, that just logs the URLs visited. Implementation-wise you would just need to hook the loading stack (say ChromeNetworkDelegate, or add a NetLog observer), and log the URL request start events (do you care about redirects too?). Your callback on URL load could do a dumb write to file with your own flushing strategy. The volume of data would be much lower than the full NetLog even stream so impact is less bad if you decided to do this synchronously (also it would be a separate command line flag which would be uncommonly used). My suggestion for using memory mapped files came from an expectation that implementations should be capable of flushing data written to the memory mapped range even after a crash has occurred. However that is supposition on my part, not sure if such crash resilience is guaranteed.

Cheers.
Or could just trivially use a proxy, assuming SSL support isn't needed.
@mmenke: I am pretty sure in their case they are more interested in file:// URLs
Status: WontFix (was: Untriaged)
Closing this as a WontFix for being a generic part of --log-net-log

Let's continue the discussion from comment #7 as a separate feature request to address your use-case (as this is simpler than a full net-log).
Hrm...  testing randomly generated file URLs seems a bit problematic, since they can freely redirect to other file URLs, and thus have implicit dependencies on random files on the test system, at least as long as Chrome has access to them.

Sign in to add a comment