Skip to content

Re-enable fcntl locking for regular files#2259

Merged
sporksmith merged 1 commit intoshadow:mainfrom
sporksmith:fcntl-locks
Jul 7, 2022
Merged

Re-enable fcntl locking for regular files#2259
sporksmith merged 1 commit intoshadow:mainfrom
sporksmith:fcntl-locks

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

This unbreaks arti's usage of sqlite. Unfortunately this implementation
is incorrect if multiple processes actually access the same file's
locks. See #2258

@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Jul 6, 2022
@sporksmith sporksmith added this to the Support arti milestone Jul 6, 2022
@sporksmith sporksmith force-pushed the fcntl-locks branch 2 times, most recently from 12a89f9 to d7e3c22 Compare July 6, 2022 22:14
@sporksmith sporksmith marked this pull request as ready for review July 6, 2022 23:50
@sporksmith sporksmith requested a review from stevenengler July 6, 2022 23:50
@stevenengler
Copy link
Copy Markdown
Contributor

Nice catch! Was there a specific error message or wrong behaviour that caused you to notice this?

This unbreaks arti's usage of sqlite. Unfortunately this implementation
is incorrect if multiple processes actually access the same file's
locks. See shadow#2258
@sporksmith
Copy link
Copy Markdown
Contributor Author

Nice catch! Was there a specific error message or wrong behaviour that caused you to notice this?

arti propagated up an error about sqlite being unable to initialize. I think I found the fcntl returning EINVAL from the strace log.

@sporksmith sporksmith enabled auto-merge (squash) July 7, 2022 16:56
@sporksmith sporksmith merged commit 6fed9e8 into shadow:main Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants