New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 792534 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
OOO until Feb 4th
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

File creation race in apkbuilder.py

Project Member Reported by alexilin@chromium.org, Dec 6 2017

Issue description

Builder failure: https://ci.chromium.org/buildbot/chromium/Android/79291
It's super flaky but it did happen.

Failure reason:
[37782/73564] ACTION //chrome/test/chromedriver/test/webview_shell:chromedriver_webview_shell_apk__create__package_apk(//build/toolchain/android:android_clang_arm)
FAILED: gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk/chromedriver_webview_shell_apk_unsigned.apk 
python ../../build/android/gyp/apkbuilder.py --depfile gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk__create__package_apk.d --resource-apk=gen/arsc/apks/ChromeDriverWebViewShell.ap_ --output-apk=gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk/chromedriver_webview_shell_apk_unsigned.apk --assets=@FileArg\(gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk.build_config:assets\) --uncompressed-assets=@FileArg\(gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk.build_config:uncompressed_assets\) --java-resources=@FileArg\(gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk.build_config:java_resources_jars\) --apk-pak-info-path size-info/ChromeDriverWebViewShell.apk.pak.info --dex-file=gen/chrome/test/chromedriver/test/webview_shell/chromedriver_webview_shell_apk/classes.dex
Traceback (most recent call last):
  File "../../build/android/gyp/apkbuilder.py", line 346, in <module>
    main(sys.argv[1:])
  File "../../build/android/gyp/apkbuilder.py", line 342, in main
    depfile_deps=depfile_deps)
  File "/b/c/b/Android/src/build/android/gyp/util/build_utils.py", line 602, in CallAndWriteDepfileIfStale
    pass_changes=True)
  File "/b/c/b/Android/src/build/android/gyp/util/md5_check.py", line 87, in CallAndRecordIfStale
    function(*args)
  File "/b/c/b/Android/src/build/android/gyp/util/build_utils.py", line 585, in on_stale_md5
    function(*args)
  File "../../build/android/gyp/apkbuilder.py", line 329, in on_stale_md5
    options.assets + options.uncompressed_assets)
  File "../../build/android/gyp/apkbuilder.py", line 199, in _MergePakInfoFiles
    os.makedirs(info_dir)
  File "/usr/lib/python2.7/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: 'size-info'

Even though we check that path doesn't exists before create a directory, it seems that someone else can create the same directory in parallel.

  # Ensure that parent dirs exist before writing new files.
  info_dir = os.path.dirname(pak_info_path)
  if not os.path.exists(info_dir):
    os.makedirs(info_dir)

Adding a try-catch block may help
 
Owner: wnwen@chromium.org
Status: Assigned (was: Untriaged)
I think the correct fix would be to remove the mkdirs() and mark the .info file as an output of the action in GN:
https://cs.chromium.org/chromium/src/build/config/android/internal_rules.gni?rcl=ad33ed3adb4ffc05903c1d4095ff264579583437&l=1729

This way ninja will create the directory before executing the script.

Comment 2 by wnwen@chromium.org, Dec 7 2017

Status: Started (was: Assigned)

Comment 3 by wnwen@chromium.org, Dec 11 2017

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 11 2017

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

commit 40a260125252553a344e5ba748c232d03285705d
Author: Peter Wen <wnwen@chromium.org>
Date: Mon Dec 11 15:40:50 2017

Android: Fix race condition in apkbuilder

Rather than create the output directory in apkbuilder.py, add it to the
gn target's output for gn to create it automatically.

Bug:  792534 
Change-Id: I3de087be4839e917b4308c38016da3b783668a96
Reviewed-on: https://chromium-review.googlesource.com/815355
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523103}
[modify] https://crrev.com/40a260125252553a344e5ba748c232d03285705d/build/android/gyp/apkbuilder.py
[modify] https://crrev.com/40a260125252553a344e5ba748c232d03285705d/build/config/android/internal_rules.gni

Sign in to add a comment