Revert "Use OS level locking in git_cache.py"
This reverts commit d3affaa624.
Reason for revert: no attribute ignore_lock
Original change's description:
> Use OS level locking in git_cache.py
>
> Without OS level locking it's possible to leave "lock" files on disk
> which will prevent next run to acquire those locks. This can easily
> happen if SIGKIL is issued.
>
> R=apolito@google.com, ehmaldonado@chromium.org
>
> Bug: 1049610
> Change-Id: Id87aa1376b9ea5ff0c2d14f3603636493ed1dd5b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2189333
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
TBR=iannucci@chromium.org,ehmaldonado@chromium.org,apolito@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com
Change-Id: Iecc963e0a99d7f59f3f8801e529839346f9fbaf3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1049610
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2211186
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
changes/86/2211186/2
Josip Sokcevic
5 years ago
committed by
LUCI CQ
parent
7aed4bb802
commit
8f6bfe301f
@ -1,116 +0,0 @@
|
||||
# Copyright 2020 The Chromium Authors. All rights reserved.
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
"""Exclusive filelocking for all supported platforms."""
|
||||
|
||||
from __future__ import print_function
|
||||
|
||||
import contextlib
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
|
||||
|
||||
class LockError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
if sys.platform.startswith('win'):
|
||||
# Windows implementation
|
||||
import win32imports
|
||||
|
||||
BYTES_TO_LOCK = 1
|
||||
|
||||
def _open_file(lockfile):
|
||||
return win32imports.Handle(
|
||||
win32imports.CreateFileW(
|
||||
lockfile, # lpFileName
|
||||
win32imports.GENERIC_WRITE, # dwDesiredAccess
|
||||
0, # dwShareMode=prevent others from opening file
|
||||
None, # lpSecurityAttributes
|
||||
win32imports.CREATE_ALWAYS, # dwCreationDisposition
|
||||
win32imports.FILE_ATTRIBUTE_NORMAL, # dwFlagsAndAttributes
|
||||
None # hTemplateFile
|
||||
))
|
||||
|
||||
def _close_file(handle):
|
||||
# CloseHandle releases lock too.
|
||||
win32imports.CloseHandle(handle)
|
||||
|
||||
def _lock_file(handle):
|
||||
ret = win32imports.LockFileEx(
|
||||
handle, # hFile
|
||||
win32imports.LOCKFILE_FAIL_IMMEDIATELY
|
||||
| win32imports.LOCKFILE_EXCLUSIVE_LOCK, # dwFlags
|
||||
0, #dwReserved
|
||||
BYTES_TO_LOCK, # nNumberOfBytesToLockLow
|
||||
0, # nNumberOfBytesToLockHigh
|
||||
win32imports.Overlapped() # lpOverlapped
|
||||
)
|
||||
# LockFileEx returns result as bool, which is converted into an integer
|
||||
# (1 == successful; 0 == not successful)
|
||||
if ret == 0:
|
||||
error_code = win32imports.GetLastError()
|
||||
raise OSError('Failed to lock handle (error code: %d).' % error_code)
|
||||
else:
|
||||
# Unix implementation
|
||||
import fcntl
|
||||
|
||||
def _open_file(lockfile):
|
||||
open_flags = (os.O_CREAT | os.O_WRONLY)
|
||||
return os.open(lockfile, open_flags, 0o644)
|
||||
|
||||
def _close_file(fd):
|
||||
os.close(fd)
|
||||
|
||||
def _lock_file(fd):
|
||||
fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
|
||||
|
||||
def _try_lock(lockfile):
|
||||
f = _open_file(lockfile)
|
||||
try:
|
||||
_lock_file(f)
|
||||
except Exception:
|
||||
_close_file(f)
|
||||
raise
|
||||
return lambda: _close_file(f)
|
||||
|
||||
|
||||
def _lock(path, timeout=0):
|
||||
"""_lock returns function to release the lock if locking was successful.
|
||||
|
||||
_lock also implements simple retry logic."""
|
||||
elapsed = 0
|
||||
while True:
|
||||
try:
|
||||
return _try_lock(path + '.locked')
|
||||
except (OSError, IOError) as e:
|
||||
if elapsed < timeout:
|
||||
sleep_time = min(10, timeout - elapsed)
|
||||
logging.info(
|
||||
'Could not create git cache lockfile; '
|
||||
'will retry after sleep(%d).', sleep_time)
|
||||
elapsed += sleep_time
|
||||
time.sleep(sleep_time)
|
||||
continue
|
||||
raise LockError("Error locking %s (err: %s)" % (path, str(e)))
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def lock(path, timeout=0):
|
||||
"""Get exclusive lock to path.
|
||||
|
||||
Usage:
|
||||
import lockfile
|
||||
with lockfile.lock(path, timeout):
|
||||
# Do something
|
||||
pass
|
||||
|
||||
"""
|
||||
release_fn = _lock(path, timeout)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
release_fn()
|
||||
@ -1,115 +0,0 @@
|
||||
#!/usr/bin/env vpython3
|
||||
# Copyright 2020 The Chromium Authors. All rights reserved.
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
"""Unit tests for lockfile.py"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import unittest
|
||||
|
||||
if sys.version_info.major == 2:
|
||||
import mock
|
||||
import Queue
|
||||
else:
|
||||
from unittest import mock
|
||||
import queue as Queue
|
||||
|
||||
DEPOT_TOOLS_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
sys.path.insert(0, DEPOT_TOOLS_ROOT)
|
||||
|
||||
from testing_support import coverage_utils
|
||||
|
||||
import lockfile
|
||||
|
||||
|
||||
class LockTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.cache_dir = tempfile.mkdtemp(prefix='lockfile')
|
||||
self.addCleanup(shutil.rmtree, self.cache_dir, ignore_errors=True)
|
||||
|
||||
def testLock(self):
|
||||
with lockfile.lock(self.cache_dir):
|
||||
# cached dir locked, attempt to lock it again
|
||||
with self.assertRaises(lockfile.LockError):
|
||||
with lockfile.lock(self.cache_dir):
|
||||
pass
|
||||
|
||||
with lockfile.lock(self.cache_dir):
|
||||
pass
|
||||
|
||||
@mock.patch('time.sleep')
|
||||
def testLockConcurrent(self, sleep_mock):
|
||||
'''testLockConcurrent simulates what happens when two separate processes try
|
||||
to acquire the same file lock with timeout.'''
|
||||
# Queues q_f1 and q_sleep are used to controll execution of individual
|
||||
# threads.
|
||||
q_f1 = Queue.Queue()
|
||||
q_sleep = Queue.Queue()
|
||||
results = Queue.Queue()
|
||||
|
||||
def side_effect(arg):
|
||||
'''side_effect is called when with l.lock is blocked. In this unit test
|
||||
case, it comes from f2.'''
|
||||
logging.debug('sleep: started')
|
||||
q_sleep.put(True)
|
||||
logging.debug('sleep: waiting for q_sleep to be consumed')
|
||||
q_sleep.join()
|
||||
logging.debug('sleep: waiting for result before exiting')
|
||||
results.get(timeout=1)
|
||||
logging.debug('sleep: exiting')
|
||||
|
||||
sleep_mock.side_effect = side_effect
|
||||
|
||||
def f1():
|
||||
'''f1 enters first in l.lock (controlled via q_f1). It then waits for
|
||||
side_effect to put a message in queue q_sleep.'''
|
||||
logging.debug('f1 started, locking')
|
||||
|
||||
with lockfile.lock(self.cache_dir, timeout=1):
|
||||
logging.debug('f1: locked')
|
||||
q_f1.put(True)
|
||||
logging.debug('f1: waiting on q_f1 to be consumed')
|
||||
q_f1.join()
|
||||
logging.debug('f1: done waiting on q_f1, getting q_sleep')
|
||||
q_sleep.get(timeout=1)
|
||||
results.put(True)
|
||||
|
||||
logging.debug('f1: lock released')
|
||||
q_sleep.task_done()
|
||||
logging.debug('f1: exiting')
|
||||
|
||||
def f2():
|
||||
'''f2 enters second in l.lock (controlled by q_f1).'''
|
||||
logging.debug('f2: started, consuming q_f1')
|
||||
q_f1.get(timeout=1) # wait for f1 to execute lock
|
||||
q_f1.task_done()
|
||||
logging.debug('f2: done waiting for q_f1, locking')
|
||||
|
||||
with lockfile.lock(self.cache_dir, timeout=1):
|
||||
logging.debug('f2: locked')
|
||||
results.put(True)
|
||||
|
||||
t1 = threading.Thread(target=f1)
|
||||
t1.start()
|
||||
t2 = threading.Thread(target=f2)
|
||||
t2.start()
|
||||
t1.join()
|
||||
t2.join()
|
||||
|
||||
# One result was consumed by side_effect, we expect only one in the queue.
|
||||
self.assertEqual(1, results.qsize())
|
||||
sleep_mock.assert_called_once_with(1)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
logging.basicConfig(
|
||||
level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)
|
||||
sys.exit(
|
||||
coverage_utils.covered_main(
|
||||
(os.path.join(DEPOT_TOOLS_ROOT, 'git_cache.py')),
|
||||
required_percentage=0))
|
||||
@ -1,61 +0,0 @@
|
||||
# Copyright 2020 The Chromium Authors. All rights reserved.
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
"""Win32 functions and constants."""
|
||||
|
||||
import ctypes
|
||||
import ctypes.wintypes
|
||||
|
||||
GENERIC_WRITE = 0x40000000
|
||||
CREATE_ALWAYS = 0x00000002
|
||||
FILE_ATTRIBUTE_NORMAL = 0x00000080
|
||||
LOCKFILE_EXCLUSIVE_LOCK = 0x00000002
|
||||
LOCKFILE_FAIL_IMMEDIATELY = 0x00000001
|
||||
|
||||
|
||||
class Overlapped(ctypes.Structure):
|
||||
"""Overlapped is required and used in LockFileEx and UnlockFileEx."""
|
||||
_fields_ = [('Internal', ctypes.wintypes.LPVOID),
|
||||
('InternalHigh', ctypes.wintypes.LPVOID),
|
||||
('Offset', ctypes.wintypes.DWORD),
|
||||
('OffsetHigh', ctypes.wintypes.DWORD),
|
||||
('Pointer', ctypes.wintypes.LPVOID),
|
||||
('hEvent', ctypes.wintypes.HANDLE)]
|
||||
|
||||
|
||||
# https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
|
||||
CreateFileW = ctypes.windll.kernel32.CreateFileW
|
||||
CreateFileW.argtypes = [
|
||||
ctypes.wintypes.LPCWSTR, # lpFileName
|
||||
ctypes.wintypes.DWORD, # dwDesiredAccess
|
||||
ctypes.wintypes.DWORD, # dwShareMode
|
||||
ctypes.wintypes.LPVOID, # lpSecurityAttributes
|
||||
ctypes.wintypes.DWORD, # dwCreationDisposition
|
||||
ctypes.wintypes.DWORD, # dwFlagsAndAttributes
|
||||
ctypes.wintypes.LPVOID, # hTemplateFile
|
||||
]
|
||||
CreateFileW.restype = ctypes.wintypes.HANDLE
|
||||
|
||||
# https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle
|
||||
CloseHandle = ctypes.windll.kernel32.CloseHandle
|
||||
CloseHandle.argtypes = [
|
||||
ctypes.wintypes.HANDLE, # hFile
|
||||
]
|
||||
CloseHandle.restype = ctypes.wintypes.BOOL
|
||||
|
||||
# https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-lockfileex
|
||||
LockFileEx = ctypes.windll.kernel32.LockFileEx
|
||||
LockFileEx.argtypes = [
|
||||
ctypes.wintypes.HANDLE, # hFile
|
||||
ctypes.wintypes.DWORD, # dwFlags
|
||||
ctypes.wintypes.DWORD, # dwReserved
|
||||
ctypes.wintypes.DWORD, # nNumberOfBytesToLockLow
|
||||
ctypes.wintypes.DWORD, # nNumberOfBytesToLockHigh
|
||||
ctypes.POINTER(Overlapped), # lpOverlapped
|
||||
]
|
||||
LockFileEx.restype = ctypes.wintypes.BOOL
|
||||
|
||||
# Commonly used functions are listed here so callers don't need to import
|
||||
# ctypes.
|
||||
GetLastError = ctypes.GetLastError
|
||||
Handle = ctypes.wintypes.HANDLE
|
||||
Loading…
Reference in New Issue