Repository: pytest-dev/pytest-services (language: python)

Issue:
`zc.lockfile.LockError` Is Raised in `lock_resource`
The switch to `zc.lockfile` in https://github.com/pytest-dev/pytest-services/pull/27 breaks our tests together with pytest-xdist.

```
@pytest.fixture(scope="session")
def _xyz_server_process(port_getter):
    port = port_getter()
```

We have
```
pytest = "6.0.2"
pytest-django = "3.10.0"
pytest-instafail = "0.4.2"
pytest-selenium = "2.0.0"
pytest-services = "2.1.0"
pytest-timeout = "1.4.2"
pytest-xdist = "2.1.0"
```

`pytest -n2` fails and `pytest` works.

```
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:59: in _lock_file
    fcntl.flock(file.fileno(), _flags)
E   BlockingIOError: [Errno 35] Resource temporarily unavailable

During handling of the above exception, another exception occurred:
tests/browser/conftest.py:195: in _xyz_server_process
    port = port_getter()
.venv/lib/python3.7/site-packages/pytest_services/locks.py:140: in get_port
    port = get_free_port(lock_dir, services_log)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:117: in get_free_port
    return lock_resource('port', get_port, lock_dir, services_log)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:85: in lock_resource
    with locked_resources(name, lock_dir) as bound_resources:
../../.pyenv/versions/3.7.2/lib/python3.7/contextlib.py:112: in __enter__
    return next(self.gen)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:56: in locked_resources
    with file_lock(os.path.join(lock_dir, name), remove=False) as fd:
../../.pyenv/versions/3.7.2/lib/python3.7/contextlib.py:112: in __enter__
    return next(self.gen)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:27: in file_lock
    with contextlib.closing(zc.lockfile.SimpleLockFile(filename)) as lockfile:
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:90: in __init__
    _lock_file(fp)
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:61: in _lock_file
    raise LockError("Couldn't lock %r" % file.name)
E   zc.lockfile.LockError: Couldn't lock '/tmp/service-locks/port'
```

Tested on macOS Catalina and Debian.

Each start of tests is not always failing but it's very often.
Handle zc.lockfile.LockError in lock_resource
Closes https://github.com/pytest-dev/pytest-services/issues/38.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/pytest-dev/pytest-services/39)
<!-- Reviewable:end -->

`zc.lockfile.LockError` Is Raised in `lock_resource`
The switch to `zc.lockfile` in https://github.com/pytest-dev/pytest-services/pull/27 breaks our tests together with pytest-xdist.

```
@pytest.fixture(scope="session")
def _xyz_server_process(port_getter):
    port = port_getter()
```

We have
```
pytest = "6.0.2"
pytest-django = "3.10.0"
pytest-instafail = "0.4.2"
pytest-selenium = "2.0.0"
pytest-services = "2.1.0"
pytest-timeout = "1.4.2"
pytest-xdist = "2.1.0"
```

`pytest -n2` fails and `pytest` works.

```
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:59: in _lock_file
    fcntl.flock(file.fileno(), _flags)
E   BlockingIOError: [Errno 35] Resource temporarily unavailable

During handling of the above exception, another exception occurred:
tests/browser/conftest.py:195: in _xyz_server_process
    port = port_getter()
.venv/lib/python3.7/site-packages/pytest_services/locks.py:140: in get_port
    port = get_free_port(lock_dir, services_log)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:117: in get_free_port
    return lock_resource('port', get_port, lock_dir, services_log)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:85: in lock_resource
    with locked_resources(name, lock_dir) as bound_resources:
../../.pyenv/versions/3.7.2/lib/python3.7/contextlib.py:112: in __enter__
    return next(self.gen)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:56: in locked_resources
    with file_lock(os.path.join(lock_dir, name), remove=False) as fd:
../../.pyenv/versions/3.7.2/lib/python3.7/contextlib.py:112: in __enter__
    return next(self.gen)
.venv/lib/python3.7/site-packages/pytest_services/locks.py:27: in file_lock
    with contextlib.closing(zc.lockfile.SimpleLockFile(filename)) as lockfile:
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:90: in __init__
    _lock_file(fp)
.venv/lib/python3.7/site-packages/zc/lockfile/__init__.py:61: in _lock_file
    raise LockError("Couldn't lock %r" % file.name)
E   zc.lockfile.LockError: Couldn't lock '/tmp/service-locks/port'
```

Tested on macOS Catalina and Debian.

Each start of tests is not always failing but it's very often.
Handle zc.lockfile.LockError in lock_resource
Closes https://github.com/pytest-dev/pytest-services/issues/38.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/pytest-dev/pytest-services/39)
<!-- Reviewable:end -->

Add zc.lockfile.LockError handling to locks.file_lock
Following @StabbarN lead for issue #38 and fix in #39, I added a timeout loop and exception handling for the exception raised by zc.lockfile.

Working on getting tox to run for your unit tests, figured I would open the PR in the meanwhile.

Testing against SatelliteQE/Robottelo unit tests where this was encountered after the update to pytest-services 2.2.0 is successful, for what its worth.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/pytest-dev/pytest-services/42)
<!-- Reviewable:end -->



Discussion / hints:
I don't think it's an incompatibility at all, it's more like a bug in the usage of zc.lockfile in the pytest-services
do you also use the latest version of zc.lockfile?
@bubenkoff Yes, we use the latest version (as specified in https://github.com/pytest-dev/pytest-services/blob/master/setup.py#L22). Yeah, I agree, sloppy title of the issue, it's not about incompatibility.

@jaraco, the author of https://github.com/pytest-dev/pytest-services/pull/27, please read this issue and https://github.com/zopefoundation/zc.lockfile/issues/23 . What's your thoughts?
@jaraco @StabbarN looks like just an incorrect exception handling when the lock cannot be obtained due to the concurrent worker
@bubenkoff My PR https://github.com/pytest-dev/pytest-services/pull/39 seems to fix the problem for us. Will you review it and if OK make a new release?
> What's your thoughts?

Thanks for looping me in, and sorry for the trouble. I'd not encountered this issue before, and it does seem the zc.lockfile maintainers have provided guidance, so your approach seems sound given the limitations.

[![Coverage Status](https://coveralls.io/builds/33673106/badge)](https://coveralls.io/builds/33673106)

Coverage decreased (-0.7%) to 71.429% when pulling **753ccdf2c3be8bca4aced50d96be624064b179c5 on StabbarN:master** into **56901122460e8af3621b8385f6bf3a19973fe884 on pytest-dev:master**.

Any more suggestions?
@StabbarN please  merge with squashing
@StabbarN 2.2.0 is out, thanks
I don't think it's an incompatibility at all, it's more like a bug in the usage of zc.lockfile in the pytest-services
do you also use the latest version of zc.lockfile?
@bubenkoff Yes, we use the latest version (as specified in https://github.com/pytest-dev/pytest-services/blob/master/setup.py#L22). Yeah, I agree, sloppy title of the issue, it's not about incompatibility.

@jaraco, the author of https://github.com/pytest-dev/pytest-services/pull/27, please read this issue and https://github.com/zopefoundation/zc.lockfile/issues/23 . What's your thoughts?
@jaraco @StabbarN looks like just an incorrect exception handling when the lock cannot be obtained due to the concurrent worker
@bubenkoff My PR https://github.com/pytest-dev/pytest-services/pull/39 seems to fix the problem for us. Will you review it and if OK make a new release?
> What's your thoughts?

Thanks for looping me in, and sorry for the trouble. I'd not encountered this issue before, and it does seem the zc.lockfile maintainers have provided guidance, so your approach seems sound given the limitations.

[![Coverage Status](https://coveralls.io/builds/33673106/badge)](https://coveralls.io/builds/33673106)

Coverage decreased (-0.7%) to 71.429% when pulling **753ccdf2c3be8bca4aced50d96be624064b179c5 on StabbarN:master** into **56901122460e8af3621b8385f6bf3a19973fe884 on pytest-dev:master**.

Any more suggestions?
@StabbarN please  merge with squashing
@StabbarN 2.2.0 is out, thanks


Fix patch:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index 3dd1f36..8873608 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -10,6 +10,7 @@ These people have contributed to `pytest-services`, in alphabetical order:
 * `Dmitrijs Milajevs <dimazest@gmail.com>`_
 * `Jason R. Coombs <jaraco@jaraco.com>`_
 * `Joep van Dijken <joepvandijken@github.com>`_
+* `Magnus Staberg <magnus@staberg.io>`_
+* `Michael Shriver <mshriver@redhat.com>`_
 * `Oleg Pidsadnyi <oleg.pidsadnyi@gmail.com>`_
 * `Zac Hatfield-Dodds <zac@zhd.dev>`_
-* `Magnus Staberg <magnus@staberg.io>`_
diff --git a/CHANGES.rst b/CHANGES.rst
index 2d10a2f..15e81c7 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+Unreleased
+-----
+
+- #42: Retry on ``zc.lockfile.LockError`` in ``file_lock``, use existing timeout kwarg
+
 2.2.0
 -----
 


Test patch:
diff --git a/pytest_services/locks.py b/pytest_services/locks.py
index 9cf20ae..8a3553a 100644
--- a/pytest_services/locks.py
+++ b/pytest_services/locks.py
@@ -24,10 +24,21 @@ def file_lock(filename, remove=True, timeout=20):
     """A lock that is shared across processes.
 
     :param filename: the name of the file that will be locked.
-
+    :param remove: whether or not to remove the file on context close
+    :param timeout: Amount of time to retry the file lock if a :class:`zc.lockfile.LockError` is hit
     """
-    with contextlib.closing(zc.lockfile.SimpleLockFile(filename)) as lockfile:
-        yield lockfile._fp
+    total_seconds_slept = 0
+    while True:
+        try:
+            with contextlib.closing(zc.lockfile.SimpleLockFile(filename)) as lockfile:
+                yield lockfile._fp
+                break
+        except zc.lockfile.LockError as err:
+            if total_seconds_slept >= timeout:
+                raise err
+        seconds_to_sleep = random() * 0.1 + 0.05
+        total_seconds_slept += seconds_to_sleep
+        time.sleep(seconds_to_sleep)
 
     remove and try_remove(filename)