Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use custom file lock functionality #314

Merged
merged 20 commits into from
Dec 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ jobs:
source .venv/bin/activate
sh tests/test_proton_resume.sh
rm -rf "$HOME/.local/share/umu" "$HOME/Games/umu" "$HOME/.local/share/Steam/compatibilitytools.d" "$HOME/.cache/umu"
- name: Test Filelock
run: |
source .venv/bin/activate
sh tests/test_flock.sh
rm -rf "$HOME/.local/share/umu" "$HOME/Games/umu" "$HOME/.local/share/Steam/compatibilitytools.d" "$HOME/.cache/umu"
- name: Test winetricks
run: |
source .venv/bin/activate
Expand Down
1 change: 0 additions & 1 deletion packaging/deb/debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ Depends:
${misc:Depends},
python3,
python3-xlib (>= 0.33),
python3-filelock (>= 3.9.0),
apparmor-profiles,
libgl1-mesa-dri:i386,
libglx-mesa0:i386
Expand Down
1 change: 0 additions & 1 deletion packaging/deb/ubuntu/control
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ Depends:
${misc:Depends},
python3,
python3-xlib (>= 0.33),
python3-filelock (>= 3.9.0),
apparmor-profiles,
libgl1-mesa-dri:i386,
libglx-mesa0:i386
Expand Down
1 change: 0 additions & 1 deletion packaging/nix/umu-launcher.nix
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ python3Packages.buildPythonPackage {
pyth1
pkgs.bubblewrap
pkgs.python3Packages.xlib
pkgs.python3Packages.filelock
pkgs.python3Packages.urllib3
] ++ lib.optional truststore pkgs.python3Packages.truststore;
makeFlags = [ "PYTHON_INTERPRETER=${pyth1}/bin/python" "SHELL_INTERPRETER=/run/current-system/sw/bin/bash" "DESTDIR=${placeholder "out"}" ];
Expand Down
1 change: 0 additions & 1 deletion packaging/rpm/umu-launcher.spec
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ BuildRequires: python3
Requires: python
Requires: python3
Requires: python3-xlib
Requires: python3-filelock


%description
Expand Down
1 change: 0 additions & 1 deletion packaging/snap/snap/snapcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ parts:
plugin: nil
stage-packages:
- python3-xlib
- python3-filelock
prime:
- usr/lib/python3

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ classifiers = [
urls = { repository = "https://github.com/Open-Wine-Components/umu-launcher" }
# Note: urllib3 is a vendored dependency. When using our Makefile, it will be
# installed automatically.
dependencies = ["python-xlib>=0.33", "filelock>=3.9.0", "urllib3>=2.0.0,<3.0.0"]
dependencies = ["python-xlib>=0.33", "urllib3>=2.0.0,<3.0.0"]

[project.optional-dependencies]
# Recommended
Expand Down
1 change: 0 additions & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
python-xlib>=0.33
filelock>=3.15.4
urllib3>=2.0.0,<3.0.0
31 changes: 31 additions & 0 deletions tests/test_flock.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env sh

#
# Ensure umu-launcher does not download its fetched resources more than once
# when multiple processes of itself are created
#

tmp1=$(mktemp)
tmp2=$(mktemp)

UMU_LOG=debug GAMEID=umu-0 "$HOME/.local/bin/umu-run" wineboot -u 2> "$tmp1" &
sleep 1
UMU_LOG=debug GAMEID=umu-0 "$HOME/.local/bin/umu-run" wineboot -u 2> "$tmp2" &
wait

grep "exited with wait status" "$tmp1" && grep -E "exited with wait status" "$tmp2"

# Ensure the 2nd proc didn't download the runtime
grep -E "\(latest\), please wait..." "$tmp2"
exit_code=$?
if "$exit_code" -ne 0; then
exit 1
fi

# Ensure the 2nd proc didn't download Proton
grep "Downloading" "$tmp2"
exit_code=$?
if "$exit_code" -ne 0; then
exit 1
fi

35 changes: 17 additions & 18 deletions umu/umu_proton.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from tempfile import TemporaryDirectory
from typing import Any

from filelock import FileLock
from urllib3.exceptions import HTTPError
from urllib3.exceptions import TimeoutError as TimeoutErrorUrllib3
from urllib3.poolmanager import PoolManager
Expand All @@ -20,6 +19,7 @@
extract_tarfile,
file_digest,
run_zenity,
unix_flock,
write_file_chunks,
)

Expand Down Expand Up @@ -313,7 +313,7 @@ def _get_latest(
proton: str
# Name of the Proton version, which is either UMU-Proton or GE-Proton
version: str
lock: FileLock
lockfile: str = f"{UMU_LOCAL}/compatibilitytools.d.lock"

if not assets:
return None
Expand All @@ -335,19 +335,19 @@ def _get_latest(

# Use the latest UMU/GE-Proton
try:
lock = FileLock(f"{UMU_LOCAL}/compatibilitytools.d.lock")
log.debug("Acquiring file lock '%s'...", lock.lock_file)
lock.acquire()

# Once acquiring the lock check if Proton hasn't been installed
if steam_compat.joinpath(proton).is_dir():
raise FileExistsError

# Download the archive to a temporary directory
_fetch_proton(env, session_caches, assets, session_pools)

# Extract the archive then move the directory
_install_proton(tarball, session_caches, steam_compat, session_pools)
log.debug("Acquiring file lock '%s'...", lockfile)
with unix_flock(lockfile):
# Once acquiring the lock check if Proton hasn't been installed
if steam_compat.joinpath(proton).is_dir():
raise FileExistsError

# Download the archive to a temporary directory
_fetch_proton(env, session_caches, assets, session_pools)

# Extract the archive then move the directory
_install_proton(
tarball, session_caches, steam_compat, session_pools
)
except (
ValueError,
KeyboardInterrupt,
Expand All @@ -356,11 +356,10 @@ def _get_latest(
log.exception(e)
return None
except FileExistsError:
# Proton was installed by another proc, continue
pass
finally:
log.debug("Released file lock '%s'", lock.lock_file)
lock.release()

log.debug("Released file lock '%s'", lockfile)
os.environ["PROTONPATH"] = str(steam_compat.joinpath(proton))
env["PROTONPATH"] = os.environ["PROTONPATH"]
log.debug("Removing: %s", tarball)
Expand Down
4 changes: 2 additions & 2 deletions umu/umu_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from subprocess import Popen
from typing import Any

from filelock import FileLock
from urllib3 import PoolManager, Retry
from urllib3.exceptions import MaxRetryError, NewConnectionError
from urllib3.exceptions import TimeoutError as TimeoutErrorUrllib3
Expand Down Expand Up @@ -54,6 +53,7 @@
get_library_paths,
has_umu_setup,
is_installed_verb,
unix_flock,
xdisplay,
)

Expand Down Expand Up @@ -830,7 +830,7 @@ def umu_run(args: Namespace | tuple[str, list[str]]) -> int:
UMU_LOCAL.mkdir(parents=True, exist_ok=True)

# Prepare the prefix
with FileLock(f"{UMU_LOCAL}/pfx.lock"):
with unix_flock(f"{UMU_LOCAL}/pfx.lock"):
setup_pfx(env["WINEPREFIX"])

# Configure the environment
Expand Down
23 changes: 12 additions & 11 deletions umu/umu_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from subprocess import run
from tempfile import TemporaryDirectory, mkdtemp

from filelock import FileLock
from urllib3.exceptions import HTTPError
from urllib3.exceptions import TimeoutError as TimeoutErrorUrllib3
from urllib3.poolmanager import PoolManager
Expand All @@ -28,6 +27,7 @@
file_digest,
has_umu_setup,
run_zenity,
unix_flock,
write_file_chunks,
)

Expand Down Expand Up @@ -476,14 +476,15 @@ def _restore_umu(
session_pools: SessionPools,
callback_fn: Callable[[], bool],
) -> None:
with FileLock(f"{local}/umu.lock") as lock:
log.debug("Acquired file lock '%s'...", lock.lock_file)
lockfile: str = f"{local}/umu.lock"
with unix_flock(lockfile):
log.debug("Acquired file lock '%s'...", lockfile)
if callback_fn():
log.debug("Released file lock '%s'", lock.lock_file)
log.debug("Released file lock '%s'", lockfile)
log.info("%s was restored", runtime_ver[1])
return
_install_umu(runtime_ver, session_pools)
log.debug("Released file lock '%s'", lock.lock_file)
log.debug("Released file lock '%s'", lockfile)


def _restore_umu_platformid(
Expand Down Expand Up @@ -547,20 +548,20 @@ def _update_umu_platform(
latest: bytes = sha256(resp.data).digest()
current: bytes = sha256(local.joinpath("VERSIONS.txt").read_bytes()).digest()
versions: Path = local.joinpath("VERSIONS.txt")
lock: FileLock = FileLock(f"{local}/umu.lock")
lockfile: str = f"{local}/umu.lock"

# Compare our version file to upstream's, updating if different
if latest != current:
log.info("Updating %s to latest...", variant)
log.debug("Acquiring file lock '%s'...", lock.lock_file)
with lock:
log.debug("Acquired file lock '%s'", lock.lock_file)
log.debug("Acquiring file lock '%s'...", lockfile)
with unix_flock(lockfile):
log.debug("Acquired file lock '%s'", lockfile)
# Once another process acquires the lock, check if the latest
# runtime has already been downloaded
if latest == sha256(versions.read_bytes()).digest():
log.debug("Released file lock '%s'", lock.lock_file)
log.debug("Released file lock '%s'", lockfile)
return
_install_umu(runtime_ver, session_pools)
log.debug("Removing: %s", runtime)
rmtree(str(runtime))
log.debug("Released file lock '%s'", lock.lock_file)
log.debug("Released file lock '%s'", lockfile)
30 changes: 30 additions & 0 deletions umu/umu_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1131,9 +1131,15 @@ def test_latest_interrupt(self):
files = (("", ""), (self.test_archive.name, ""))
tmpdirs = (self.test_cache, self.test_cache_home)

# Mock the context manager object that creates the file lock
mock_ctx = MagicMock()
mock_ctx.__enter__ = MagicMock(return_value=None)
mock_ctx.__exit__ = MagicMock(return_value=None)

with (
patch("umu.umu_proton._fetch_proton") as mock_function,
ThreadPoolExecutor(),
patch.object(umu_proton, "unix_flock", return_value=mock_ctx),
):
# Mock the interrupt
# We want the dir we tried to extract to be cleaned
Expand Down Expand Up @@ -1164,6 +1170,11 @@ def test_latest_val_err(self):
files = (("", ""), (self.test_archive.name, ""))
tmpdirs = (self.test_cache, self.test_cache_home)

# Mock the context manager object that creates the file lock
mock_ctx = MagicMock()
mock_ctx.__enter__ = MagicMock(return_value=None)
mock_ctx.__exit__ = MagicMock(return_value=None)

self.assertTrue(
self.test_archive.is_file(),
"Expected test file in cache to exist",
Expand All @@ -1172,6 +1183,7 @@ def test_latest_val_err(self):
with (
patch("umu.umu_proton._fetch_proton") as mock_function,
ThreadPoolExecutor(),
patch.object(umu_proton, "unix_flock", return_value=mock_ctx),
):
# Mock the interrupt
mock_function.side_effect = ValueError
Expand All @@ -1195,11 +1207,17 @@ def test_latest_offline(self):
files = ()
tmpdirs = (self.test_cache, self.test_cache_home)

# Mock the context manager object that creates the file lock
mock_ctx = MagicMock()
mock_ctx.__enter__ = MagicMock(return_value=None)
mock_ctx.__exit__ = MagicMock(return_value=None)

os.environ["PROTONPATH"] = ""

with (
patch("umu.umu_proton._fetch_proton"),
ThreadPoolExecutor(),
patch.object(umu_proton, "unix_flock", return_value=mock_ctx),
):
result = umu_proton._get_latest(
self.env,
Expand Down Expand Up @@ -1228,6 +1246,11 @@ def test_link_umu(self):
files = ((f"{latest}.sha512sum", ""), (f"{latest}.tar.gz", ""))
tmpdirs = (self.test_cache, self.test_cache_home)

# Mock the context manager object that creates the file lock
mock_ctx = MagicMock()
mock_ctx.__enter__ = MagicMock(return_value=None)
mock_ctx.__exit__ = MagicMock(return_value=None)

# Mock the latest Proton in /tmp
test_archive = self.test_cache.joinpath(f"{latest}.tar.gz")
with tarfile.open(test_archive.as_posix(), "w:gz") as tar:
Expand All @@ -1245,6 +1268,7 @@ def test_link_umu(self):
with (
patch("umu.umu_proton._fetch_proton"),
ThreadPoolExecutor(),
patch.object(umu_proton, "unix_flock", return_value=mock_ctx),
):
result = umu_proton._get_latest(
self.env,
Expand Down Expand Up @@ -1287,6 +1311,11 @@ def test_latest_umu(self):
files = ((f"{latest}.sha512sum", ""), (f"{latest}.tar.gz", ""))
tmpdirs = (self.test_cache, self.test_cache_home)

# Mock the context manager object that creates the file lock
mock_ctx = MagicMock()
mock_ctx.__enter__ = MagicMock(return_value=None)
mock_ctx.__exit__ = MagicMock(return_value=None)

# Mock the latest Proton in /tmp
test_archive = self.test_cache.joinpath(f"{latest}.tar.gz")
with tarfile.open(test_archive.as_posix(), "w:gz") as tar:
Expand All @@ -1310,6 +1339,7 @@ def test_latest_umu(self):
with (
patch("umu.umu_proton._fetch_proton"),
ThreadPoolExecutor() as thread_pool,
patch.object(umu_proton, "unix_flock", return_value=mock_ctx),
):
result = umu_proton._get_latest(
self.env,
Expand Down
17 changes: 17 additions & 0 deletions umu/umu_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import sys
from contextlib import contextmanager
from ctypes.util import find_library
from fcntl import LOCK_EX, LOCK_UN, flock
from functools import lru_cache
from hashlib import new as hashnew
from io import BufferedIOBase
Expand All @@ -19,6 +20,22 @@
from umu.umu_log import log


@contextmanager
def unix_flock(path: str):
"""Create a file and configure it to be locking."""
fd: int | None = None

try:
fd = os.open(path, os.O_CREAT | os.O_WRONLY, 0o644)
# See https://man7.org/linux/man-pages/man2/flock.2.html
flock(fd, LOCK_EX)
yield fd
finally:
if fd is not None:
flock(fd, LOCK_UN)
os.close(fd)


@lru_cache
def get_libc() -> str:
"""Find libc.so from the user's system."""
Expand Down
Loading