528911 - lockFile should catch IOError when open()ing lock file

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

We hit a build error where JarMaker failed with a Python exception related to handling the lock file:
Traceback (most recent call last):
  File "/builds/slave/mozilla-central-macosx/build/config/JarMaker.py", line 498, in <module>
    main()
  File "/builds/slave/mozilla-central-macosx/build/config/JarMaker.py", line 495, in main
    localedirs=options.l10n_src)
  File "/builds/slave/mozilla-central-macosx/build/config/JarMaker.py", line 286, in makeJars
    jardir=jardir)
  File "/builds/slave/mozilla-central-macosx/build/config/JarMaker.py", line 231, in makeJar
    localedirs)
  File "/builds/slave/mozilla-central-macosx/build/config/JarMaker.py", line 316, in processJarSection
    jf = ZipFile(jarfilepath, 'a', lock = True)
  File "/builds/slave/mozilla-central-macosx/build/config/MozZipFile.py", line 55, in __init__
    self.lockfile = lockFile(file + '.lck')
  File "/builds/slave/mozilla-central-macosx/build/config/utils.py", line 77, in lockFile
    f = open(lockfile, "r")
IOError: [Errno 2] No such file or directory: '../../../dist/bin/chrome/browser.jar.lck'

It sounds like this is just a race condition, where the lock file existed, but was removed before it could be opened (to read the owner PID). lockFile attempts to handle this situation by catching an OSError exception, and checking if it's ENOENT, but an IOError is what's actually raised here:
http://mxr.mozilla.org/mozilla-central/source/config/utils.py#79
The Python docs say IOError is what's supposed to be raised, so this must just be a subtle race bug:
http://docs.python.org/library/functions.html#open

Comment 1

[Axel Hecht [:Pike]]

Attachment: fix race condition

Both OSError and IOError are EnvironmentError, so I just needed to make that more generic. http://docs.python.org/library/exceptions.html#exceptions.EnvironmentError doesn't indicate that that's new.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 412678 [details] [diff] [review]
fix race condition

http://hg.mozilla.org/mozilla-central/rev/92cce7e8fa37

Comment 4

[(not currently active) Ted Mielczarek]

FWIW, I would be willing to bet we only started seeing these recently because I landed bug 462381 yesterday.

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 412678 [details] [diff] [review]
fix race condition

Requesting approval for 1.9.2

This is a correctness patch for our build infra, I think we should take that independent of whether we take the PARALLEL_DIRS sprinkle or not.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 412678 [details] [diff] [review]
fix race condition

I agree. Minimal impact on the actual product, but fixes a real race condition in the build system.

Comment 7

[Axel Hecht [:Pike]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/14493b3dd480, landed on branch.