Use a lock owned by the open file descriptor on Android - #1265
Open
nekohasekai wants to merge 1 commit into
Open
Use a lock owned by the open file descriptor on Android#1265nekohasekai wants to merge 1 commit into
nekohasekai wants to merge 1 commit into
Conversation
bolt_android.go locks with POSIX record locks. A POSIX record lock belongs to the process, not to the open file descriptor. A second Open of the same database from one process therefore succeeds, and it replaces the lock of the first Open. A close of any descriptor to the file also releases every lock that the process holds on that file, which loses exclusion between processes as well. PR etcd-io#571 replaced flock(2) because it fails with ENOSYS on Android. It fails only on the FUSE mount that provides emulated external storage. The MediaProvider FUSE daemon advertises FUSE_CAP_FLOCK_LOCKS, but it does not implement the flock handler. libfuse therefore answers with ENOSYS, and fuse_file_flock() returns that reply instead of a local fallback. Storage in the app's own data directory is not affected. Use flock(2). On ENOSYS, use an open file description lock instead. An OFD lock belongs to the open file description. A second Open therefore conflicts, and an unrelated close leaves the lock in place. OFD locks also work on that mount. Storage that implements flock(2) continues to use it, and behaves as on every other unix platform. Verified on an Android 14 emulator inside an app process, against an ext4 app data directory and a FUSE-backed external directory. Signed-off-by: 世界 <i@sekai.icu>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nekohasekai The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the file lock on Android. The lock must exclude a second
Openof thesame database. At present it does not.
What is wrong today
PR #571 moved Android out of
bolt_unix.goand intobolt_android.go, to workaround
flock(2)failures on Android. The new file locks with POSIX recordlocks (
fcntl(F_SETLK)), copied frombolt_solaris.go. A POSIX record lockbelongs to the process, not to the open file descriptor. The lock therefore
loses the two properties that it exists to provide.
Measured on an Android 14 emulator (API 34, kernel 6.1.23), inside a real app
process:
Openin the same process succeeds. TwoDBhandles then writemeta pages and freelist pages to one file. This is the corruption that the
lock must prevent.
holds on that file. Another process can then take the write lock. Exclusion
between processes is the property that
Opendocuments, and it is lost too.Why
flock(2)failedAndroid does not remove or restrict
flock.flockfails only on the FUSEmount that provides emulated external storage. In
pf_init(), the MediaProviderFUSE daemon adds
FUSE_CAP_FLOCK_LOCKStoconn->want. Butpf_flockand the.flockentry in the ops table are commented out. libfuse therefore repliesENOSYSindo_setlk_common.fc->no_flockis 0, sofuse_file_flock()returns that reply without change. There is no local fallback.
The daemon does not advertise
FUSE_CAP_POSIX_LOCKS.fcntllocks thereforeuse the
posix_lock_file()path and stay in the kernel. This is whyfcntlappeared to work.
Same emulator, same app process, two directories:
/data/user/0/<pkg>/files(ext4)/storage/emulated/0/Android/data/<pkg>(FUSE)flock(2)fcntl(F_SETLK)fcntl(F_OFD_SETLK)The failure depends on the filesystem, not on the Android release. It does not
affect a database in the app's own data directory.
This change
Use
flock(2). OnENOSYS, use an open file description lock (F_OFD_SETLK)instead. An OFD lock belongs to the open file description, as a
flocklockdoes. Both properties above therefore hold.
funlockmakes the same choice.The choice is stable for a given file, because it is a property of the
filesystem.
Storage that implements
flock(2)continues to use it, and behaves as on everyother unix platform.
F_OFD_SETLKrequires Linux 3.15. The fallback path alwayshas it, because the path is reached only on the FUSE mount. Android 11
introduced that mount with a 4.14 or later kernel.
Testing
Added
TestOpen_ErrTimeout_SameProcess. It skips on aix and solaris, whichstill lock with POSIX record locks.
Verified on an Android 14 emulator (API 34, arm64). A probe and this package's
test binary ran from a real zygote-spawned app process (
Seccomp: 2), on bothfilesystems:
The full test suite of the package also passes on that emulator, run from the
app's data directory.
No other platform changes:
bolt_android.gobuilds only forGOOS=android.