Skip to content

lib: add file locking to downloads #675

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.

Sign up for GitHub

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

Open
Duncaen wants to merge 2 commits into
base: master
Choose a base branch
from
+99 −45
Open

lib: add file locking to downloads #675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f186933
lib: add file locking to downloads
Duncaen Feb 17, 2026
328f569
tests: rewrite xbps-fetch tests to use atf_check
Duncaen Feb 17, 2026
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
111 changes: 85 additions & 26 deletions lib/download.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
* From FreeBSD fetch(8):
* $FreeBSD: src/usr.bin/fetch/fetch.c,v 1.84.2.1 2009/08/03 08:13:06 kensmith Exp $
*/
#include <sys/file.h>
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/time.h>
Expand Down Expand Up @@ -94,19 +95,64 @@
return fetchLastErrString;
}

static int
acquire_lock(char *lockfile, size_t lockfilesz, const char *filename)
{
int fd;
int r;

r = snprintf(lockfile, lockfilesz, "%s.lock", filename);
if (r < 0 || (size_t)r >= lockfilesz) {
errno = ENAMETOOLONG;
return -1;
}

fd = open(lockfile, O_RDWR|O_CREAT|O_CLOEXEC, 0600);
if (fd == -1)
return -errno;

if (flock(fd, LOCK_EX|LOCK_NB) == -1) {
if (errno == EWOULDBLOCK)
xbps_warn_printf(
"%s: file locked, waiting...\n", filename);
if (errno != EWOULDBLOCK || flock(fd, LOCK_EX) == -1) {
r = -errno;
(void) close(fd);
return r;
}
}

return fd;
}

static void
release_lock(const char *lockfile, int lockfd)
{
(void) close(lockfd);
if (remove(lockfile) == -1 && errno != ENOENT) {
xbps_warn_printf("failed to remove lock file: %s: %s\n",
lockfile, strerror(errno));
}
}

int
xbps_fetch_file_dest_sha256(struct xbps_handle *xhp, const char *uri, const char *filename, const char *flags, unsigned char *digest, size_t digestlen)
xbps_fetch_file_dest_sha256(struct xbps_handle *xhp, const char *uri,
const char *filename, const char *flags, unsigned char *digest,
size_t digestlen)
{
struct stat st, st_tmpfile, *stp;
char buf[BUFSIZ];
char tempfile[PATH_MAX];
char lockfile[PATH_MAX];
struct stat st = {0}, st_tmpfile = {0}, *stp;
struct url *url = NULL;
struct url_stat url_st;
struct fetchIO *fio = NULL;
struct timespec ts[2];
off_t bytes_dload = 0;
ssize_t bytes_read = 0, bytes_written = 0;
char buf[4096], *tempfile = NULL;
char fetch_flags[8];
int fd = -1, rv = 0;
int lockfd = -1;
bool refetch = false, restart = false;
SHA256_CTX sha256;

Expand All @@ -132,35 +178,47 @@
if (flags != NULL)
xbps_strlcpy(fetch_flags, flags, 7);

tempfile = xbps_xasprintf("%s.part", filename);
/*
* Check if we have to resume a transfer.
*/
memset(&st_tmpfile, 0, sizeof(st_tmpfile));
if (stat(tempfile, &st_tmpfile) == 0) {
if (st_tmpfile.st_size > 0)
restart = true;
} else {
if (errno != ENOENT) {
rv = -1;
goto fetch_file_out;
}
rv = snprintf(tempfile, sizeof(tempfile), "%s.part", filename);
if (rv < 0 || (size_t)rv >= sizeof(tempfile)) {
errno = ENAMETOOLONG;
return -1;
}

lockfd = acquire_lock(lockfile, sizeof(lockfile), filename);
if (lockfd < 0) {
xbps_error_printf("failed to lock file: %s: %s\n", filename,
strerror(-lockfd));
return -1;
}

/*
* Check if we have to refetch a transfer.
*/
memset(&st, 0, sizeof(st));
if (stat(filename, &st) == 0) {
refetch = true;
url->last_modified = st.st_mtime;
xbps_strlcat(fetch_flags, "i", sizeof(fetch_flags));
} else {
if (errno != ENOENT) {
rv = -1;
goto fetch_file_out;
}
} else if (errno != ENOENT) {
xbps_error_printf("failed to stat target file: %s: %s\n",
filename, strerror(errno));
rv = -1;
goto fetch_file_out;
}

/*
* Check if we have to resume a transfer.
*/
if (stat(tempfile, &st_tmpfile) == 0) {
if (st_tmpfile.st_size > 0)
restart = true;
} else if (errno != ENOENT) {
xbps_error_printf("failed to stat temporary file: %s: %s\n",
tempfile, strerror(errno));
rv = -1;
goto fetch_file_out;
}

if (refetch && !restart) {

Check failure

Code scanning / CodeQL

Time-of-check time-of-use filesystem race condition High

The
filename
Loading
being operated upon was previously
checked
Loading
, but the underlying file may have been changed since then.
The
filename
Loading
being operated upon was previously
checked
Loading
, but the underlying file may have been changed since then.
/* fetch the whole file, filename available */
stp = &st;
} else {
Expand All @@ -184,6 +242,7 @@
if (fio == NULL) {
if (fetchLastErrCode == FETCH_UNCHANGED) {
/* Last-Modified matched */
rv = 0;
goto fetch_file_out;
} else if (fetchLastErrCode == FETCH_PROTO && url_st.size == stp->st_size) {
/* 413, requested offset == length */
Expand Down Expand Up @@ -312,7 +371,8 @@
rename_file:
/* File downloaded successfully, rename to destfile */
if (rename(tempfile, filename) == -1) {
xbps_dbg_printf("failed to rename %s to %s: %s",
xbps_error_printf(
"failed to rename temporary file: %s: to: %s: %s\n",
tempfile, filename, strerror(errno));
rv = -1;
goto fetch_file_out;
Expand All @@ -329,9 +389,8 @@
(void)close(fd);
if (url != NULL)
fetchFreeURL(url);

free(tempfile);

if (lockfd != -1)
release_lock(lockfile, lockfd);

Check warning

Code scanning / CodeQL

Comparison result is always the same Warning

Comparison is always true because lockfd >= 0.
Copy link
Member Author

@Duncaen Duncaen Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intended in case locking becomes optional, doesn't really make sense for every download.

return rv;
}

Expand Down
33 changes: 14 additions & 19 deletions tests/xbps/xbps-fetch/fetch_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ success_head() {
success_body() {
mkdir some_repo
touch some_repo/pkg_A
xbps-fetch file://$PWD/some_repo/pkg_A
atf_check_equal $? 0
atf_check -o ignore -- xbps-fetch file://$PWD/some_repo/pkg_A
}

atf_test_case pkgnotfound
Expand All @@ -21,8 +20,9 @@ pkgnotfound_head() {
}

pkgnotfound_body() {
xbps-fetch file://$PWD/nonexistant
atf_check_equal $? 1
atf_check -s exit:1 \
-e match:"ERROR: xbps-fetch: failed to fetch: file://${PWD}/nonexistant" \
-- xbps-fetch file://$PWD/nonexistant
}

atf_test_case identical
Expand All @@ -35,9 +35,9 @@ identical_body() {
mkdir some_repo
echo 'content' > some_repo/pkg_A
echo 'content' > pkg_A
output=$(xbps-fetch file://$PWD/some_repo/pkg_A 2>&1)
atf_check_equal $? 0
atf_check_equal "$output" "WARNING: xbps-fetch: file://$PWD/some_repo/pkg_A: file is identical with remote."
atf_check \
-e inline:"WARNING: xbps-fetch: file://$PWD/some_repo/pkg_A: file is identical with remote.\n" \
-- xbps-fetch file://$PWD/some_repo/pkg_A
}

atf_test_case multiple_success
Expand All @@ -49,12 +49,9 @@ multiple_success_head() {
multiple_success_body() {
mkdir some_repo
touch some_repo/pkg_A some_repo/pkg_B
xbps-fetch file://$PWD/some_repo/pkg_A file://$PWD/some_repo/pkg_B
atf_check_equal $? 0
test -f pkg_A
atf_check_equal $? 0
test -f pkg_B
atf_check_equal $? 0
atf_check -o ignore -- xbps-fetch file://$PWD/some_repo/pkg_A file://$PWD/some_repo/pkg_B
atf_check -- test -f pkg_A
atf_check -- test -f pkg_B
}

atf_test_case multiple_notfound
Expand All @@ -66,12 +63,10 @@ multiple_notfound_head() {
multiple_notfound_body() {
mkdir some_repo
touch some_repo/pkg_A
xbps-fetch file://$PWD/some_repo/nonexistant file://$PWD/some_repo/pkg_A
atf_check_equal $? 1
test -f pkg_A
atf_check_equal $? 0
test -f nonexistant
atf_check_equal $? 1
atf_check -s exit:1 -e match:"failed to fetch" -o ignore \
-- xbps-fetch file://$PWD/some_repo/nonexistant file://$PWD/some_repo/pkg_A
atf_check -- test -f pkg_A
atf_check -s exit:1 -- test -f nonexistant
}

atf_init_test_cases() {
Expand Down
Loading