From 4d8627e87285ead6045bcddbf70cc51183df7904 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 26 Jan 2023 15:08:55 +0100 Subject: [PATCH] fix: open the file lock on "UNIX" with O_RDRW (#791) For some reason, the file lock on UNIX was always opened using O_RDONLY. This doesn't make sense in combination with O_TRUNC, since this is a write operation. On some systems (customer-scenario was AWS EFS) this will lead to an EBADF if we try to flock that descriptor. The docs consider this combination undefined. This change also gets rid of the AIX compile conditional, since this should work on all systems now. I also added a warning to the run-initialization in case we get an error when trying to lock. macOS-11 CI runner needed to be updated to llvm15: https://github.com/actions/runner-images/blob/macOS-11/20230117.2/images/macos/macos-11-Readme.md --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 1 + src/path/sentry_path_unix.c | 10 +--------- src/sentry_database.c | 16 ++++++++++++---- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca414278f..00909a062 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,7 +134,7 @@ jobs: - name: Expose llvm PATH for Mac if: ${{ runner.os == 'macOS' }} - run: echo $(brew --prefix llvm@14)/bin >> $GITHUB_PATH + run: echo $(brew --prefix llvm@15)/bin >> $GITHUB_PATH - name: Installing Android SDK Dependencies if: ${{ env['ANDROID_API'] }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 20ae0d873..78b9c8269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Fixes**: +- Open the database file-lock on "UNIX" with `O_RDRW` ([#791](https://github.com/getsentry/sentry-native/pull/791)) - Better error messages in `sentry_transport_curl`. ([#777](https://github.com/getsentry/sentry-native/pull/777)) - Fix sporadic crash on Windows due to race condition when initializing background-worker thread-id. ([#785](https://github.com/getsentry/sentry-native/pull/785)) - Increased curl headers buffer size to 512 (in `sentry_transport_curl`). ([#784](https://github.com/getsentry/sentry-native/pull/784)) diff --git a/src/path/sentry_path_unix.c b/src/path/sentry_path_unix.c index 041a23b41..d75a0eb8f 100644 --- a/src/path/sentry_path_unix.c +++ b/src/path/sentry_path_unix.c @@ -54,15 +54,7 @@ sentry__filelock_try_lock(sentry_filelock_t *lock) { lock->is_locked = false; - const int oflags = -#ifdef SENTRY_PLATFORM_AIX - // Under AIX, O_TRUNC can only be set if it can be written to, and - // flock (well, fcntl) will return EBADF if the fd is not read-write. - O_RDWR | O_CREAT | O_TRUNC; -#else - O_RDONLY | O_CREAT | O_TRUNC; -#endif - int fd = open(lock->path->path, oflags, + int fd = open(lock->path->path, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); if (fd < 0) { return false; diff --git a/src/sentry_database.c b/src/sentry_database.c index 62a619d8a..d9874fcdb 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -4,6 +4,7 @@ #include "sentry_json.h" #include "sentry_options.h" #include "sentry_session.h" +#include #include sentry_run_t * @@ -49,13 +50,20 @@ sentry__run_new(const sentry_path_t *database_path) run->run_path = run_path; run->session_path = session_path; run->lock = sentry__filelock_new(lock_path); - if (!run->lock || !sentry__filelock_try_lock(run->lock)) { - sentry__run_free(run); - return NULL; + if (!run->lock) { + goto error; + } + if (!sentry__filelock_try_lock(run->lock)) { + SENTRY_WARNF("failed to lock file \"%s\" (%s)", lock_path->path, + strerror(errno)); + goto error; } - sentry__path_create_dir_all(run->run_path); return run; + +error: + sentry__run_free(run); + return NULL; } void