New issue
Advanced search Search tips

Issue 712456 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 655123
Owner: ----
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug



Sign in to add a comment

ipc/ipc.mojom.h is not generated when needed

Project Member Reported by brucedaw...@chromium.org, Apr 17 2017

Issue description

gen/ipc/ipc.mojom.h and gen/ipc/ipc.mojom-shared.h are both included by chrome/browser/ui/views/tabs/tab_renderer_data.cc but are not guaranteed to be generated before it is compiled. This occurs with 100% reliability on Windows and Linux if you use these commands (replace ^^ with ^ on Linux):

> gn gen out/test
> gn clean out/test
> ninja -C out/test ../../chrome/browser/ui/views/tabs/tab_renderer_data.cc^^

The error is:
In file included from ../../chrome/browser/ui/views/tabs/tab_renderer_data.cc:5:
In file included from ../../chrome/browser/ui/views/tabs/tab_renderer_data.h:9:
In file included from ../../chrome/browser/ui/tabs/tab_utils.h:13:
In file included from ../../content/public/browser/web_contents_user_data.h:10:
In file included from ../../content/public/browser/web_contents.h:22:
In file included from ../../content/public/browser/page_navigator.h:18:
In file included from ../../content/public/common/child_process_host.h:13:
In file included from ../../ipc/ipc_channel_proxy.h:20:
../../ipc/ipc_channel.h:21:10: fatal error: 'ipc/ipc.mojom.h' file not found
#include "ipc/ipc.mojom.h"
         ^~~~~~~~~~~~~~~~~
1 error generated.

You can push past the problem by forcibly generating the two missing headers:

> ninja -C out/test gen/ipc/ipc.mojom.h
> ninja -C out/test gen/ipc/ipc.mojom-shared.h
> ninja -C out/test ../../chrome/browser/ui/views/tabs/tab_renderer_data.cc^^

You can also push past the problem by building a larger target such as chrome. It will eventually generate the correct header files. If you do this with -k0 then the build will fail the first time but will certainly work the second time.

The build failures are actually the least scary aspect of this problem. The scary problem is that this behavior implies a race condition that means that stale versions of ipc.mojom.h are sometimes used, perhaps frequently. Although, it may be that this is a non-issue because once ninja knows that tab_renderer_data.cc includes these header files it can schedule their generation ahead of their use - it is probably only on clean builds that failures can occur, which is why this problem is not seen more frequently.

TL;DR - with a fresh output directory ninja doesn't know that tab_renderer_data.cc includes two generated mojo files so it doesn't generate them.

 
I confirmed that after a successful build of tab_renderer_data.cc if you delete gen\ipc\ipc.mojom.h and then rebuild tab_renderer_data.cc then gen\ipc\ipc.mojom.h is regenerated.

So, once tab_renderer_data.cc has been built once then ninja knows its dependencies and knows to generate them. Thus the problem is that there is a race condition on the first build of tab_renderer_data.cc because its inputs are unknown *and* do not necessarily exist.

I just hit another variant of this bug. There is nothing to guarantee that eventlog_messages.h is built before chrome_watcher_main.obj on a clean build. On an incremental build ninja knows about the dependency and does the right thing. Is there some generic mechanism in gn or ninja that we need to use in order to solve these problems?

The output below follows a clean build of the 'chrome' target that failed due to eventlog_messages.h not being found. I then manually retried the build of chrome_watcher_main.obj to confirm that doing so didn't cause eventlog_message.h to be generated. I then did a full rebuild of chrome which worked this time. Then I deleted eventlog_message.h and then built chrome_watcher_main.obj again - this time eventlog_message.h was regenerated:

>ninja -C out\BuildTest obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj
        ninja.exe -C out\BuildTest obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj -j 800 -l 40  -d keeprsp
ninja: Entering directory `out\BuildTest'
[1 processes, 1/1 @ 0.3/s : 3.924s ] CXX obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj
FAILED: obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj
C:\src\goma\goma-win64/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes
...
/Foobj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj /Fd"obj/chrome/chrome_watcher/chrome_watcher_cc.pdb"
../../chrome/chrome_watcher/chrome_watcher_main.cc(47,10):  fatal error: 'chrome/common/win/eventlog_messages.h' file not found
#include "chrome/common/win/eventlog_messages.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

>ninja -C out\BuildTest chrome
        ninja.exe -C out\BuildTest chrome -j 800 -l 40  -d keeprsp
ninja: Entering directory `out\BuildTest'
[1 processes, 23532/23532 @ 33.0/s : 713.296s ] STAMP obj/chrome/chrome.stamp


>del out\BuildTest\gen\chrome\common\win\eventlog_messages.h

>ninja -v -j 1 -C out\BuildTest obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj
        ninja.exe -v -j 1 -C out\BuildTest obj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj -l 40  -d keeprsp
ninja: Entering directory `out\BuildTest'
[1 processes, 1/2 @ 3.9/s : 0.259s ] c:/src/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/win/message_compiler.py environment.x86 -h gen/chrome/common/win -r gen/chrome/common/win -u ../../chrome/common/win/eventlog_messages.mc
[1 processes, 2/2 @ 0.5/s : 3.903s ] C:\src\goma\goma-win64/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes
...
/Foobj/chrome/chrome_watcher/chrome_watcher/chrome_watcher_main.obj /Fd"obj/chrome/chrome_watcher/chrome_watcher_cc.pdb"

If you want a list of all the instances where this can happen, see issue 655123 where I have a patched ninja to look for them.
Thanks for the information and that log. FWIW I don't see ipc.mojom.h or eventlog_messages.h in the most recent dependency list on bug 655123. Any ideas why? If the tool from that bug can find all of the missing dependencies then we can close this bug as a duplicate.
I only run the tool on Linux, and not on today's master, so maybe that's why?

On Linux in the commit I referenced, chrome_watcher is not built at all, and  ipc/ipc_channel_proxy.h does not include ipc/ipc.mojom.h directly; ../chrome/browser/ui/views/tabs/tab_renderer_data.cc^ built successfully in a clean outdir without an error.

It'll take me a sec to re-check after a pull because chromium without goma is pain. May be quicker to build the patched ninja on your end if you're itching to find out.

I can't get the tab_renderer_data.c / ipc.mojom.h issue to reproduce. I synced to a546ed3dd2df63b75781c82db85d0640ec7d5c66, and:

$ gn clean out2
$ ninja -C out2 ../chrome/browser/ui/views/tabs/tab_renderer_data.cc^
ninja: Entering directory `out2'
[1/1] Regenerating ninja files
[7436/7436] CXX obj/chrome/browser/ui/views/views/tab_renderer_data.o
$

(I got the include stack a bit wrong and it's ipc_channel.h that includes the mojo header, and does so for me, but that header gets generated in time anyway)
Maybe the ipc/ipc.mojom.h issue is fixed now - it was almost a year ago that I hit it, whereas the eventlog_messages.h issue is current.
Oh, right, that's likely then. I've no way to do a windows build, so can't verify that the tool catches the eventlog_messages.h but I'd expect it to.
Mergedinto: 655123
Status: Duplicate (was: Untriaged)

Sign in to add a comment