Comments (5)
by contract, do calls to load need to be protected by the storage lock?
No; this synchronization should be handled primitively, natively/internally, within the storage mechanism. The user shouldn't have to lock for every other storage operation.
from what I see, the lock is used only to protect for simultaneous writes. it's not used to lock/protect the reader if a writer is currently writing.
Locking is for user-scope purposes, like renewing certificates or something. So it can protect reads and writes, but what it's really for is to sync a certain operation or task that may involve multiple reads or writes.
if reader reads while writefile is running. it could read after open file, which truncates the file, so it will read empty.
It sounds like you know a lot more about the primitives of file systems than I do 😅 Especially after our conversation in slack. We can implement improvements to the FileStorage if needed! Is it a matter of calling Sync() in more places?
i think the easiest solution is to do what containerd is doing.
instead of writing over a file, we should write a new file and attempt to copy over it.
i will try to replicate the race condition, create a fix, and show that the race is resolved.
the comment in their code describes:
Package atomicfile provides a mechanism (on Unix-like platforms) to present a consistent view of a file to separate
processes even while the file is being written. This is accomplished by writing a temporary file, syncing to disk, and
renaming over the destination file name.
Partial/inconsistent reads can occur due to:
1. A process attempting to read the file while it is being written to (both in the case of a new file with a
short/incomplete write or in the case of an existing, updated file where new bytes may be written at the beginning
but old bytes may still be present after).
2. Concurrent goroutines leading to multiple active writers of the same file.
the lock existing lock can deal with #2, we just need a mechanism to deal with #1
from certmagic.
Filestore uses os.WriteFile
// WriteFile writes data to the named file, creating it if necessary.
// If the file does not exist, WriteFile creates it with permissions perm (before umask);
// otherwise WriteFile truncates it before writing, without changing permissions.
// Since WriteFile requires multiple system calls to complete, a failure mid-operation
// can leave the file in a partially written state.
func WriteFile(name string, data []byte, perm FileMode) error {
f, err := OpenFile(name, O_WRONLY|O_CREATE|O_TRUNC, perm)
if err != nil {
return err
}
_, err = f.Write(data)
if err1 := f.Close(); err1 != nil && err == nil {
err = err1
}
return err
}
if reader reads while writefile is running. it could read after open file, which truncates the file, so it will read empty.
so I think there is a race and protection is needed.
within a process, read will force page cache flush if read is run after write before close, however between multiple processes. the file is empty from the opening of the file to fsync on close.
within the process, an rwmutex is needed likely. however between multiple processes, it may be prudent to require a global lock on read. if so, it may be correct to switch to a locking system with rw support in the long term (advisory lock, etc)
from certmagic.
https://github.com/containerd/containerd/blob/main/pkg%2Fatomicfile%2Ffile.go
maybe can do what containerd does
from certmagic.
by contract, do calls to load need to be protected by the storage lock?
No; this synchronization should be handled primitively, natively/internally, within the storage mechanism. The user shouldn't have to lock for every other storage operation.
from what I see, the lock is used only to protect for simultaneous writes. it's not used to lock/protect the reader if a writer is currently writing.
Locking is for user-scope purposes, like renewing certificates or something. So it can protect reads and writes, but what it's really for is to sync a certain operation or task that may involve multiple reads or writes.
if reader reads while writefile is running. it could read after open file, which truncates the file, so it will read empty.
It sounds like you know a lot more about the primitives of file systems than I do 😅 Especially after our conversation in slack. We can implement improvements to the FileStorage if needed! Is it a matter of calling Sync() in more places?
from certmagic.
Related Issues (20)
- Support zerossl IP cert HOT 3
- Support customizable certificate validity period HOT 2
- Add: Deactivating an Authorization (7.5.2) HOT 4
- Certificate Import HOT 16
- Add proxy option for OCSP stapling requests HOT 6
- Ability to disable logs with `no information found to solve challenge for identifier` HOT 3
- Config option for what the Caddy ask endpoint protects / DecisionFunc HOT 2
- Can DNS be used alongside ALPN? HOT 5
- How to manually issue a certificate HOT 3
- Is FallbackServerName still experimental? HOT 3
- Question: How to issue wildcard certificates rather than exact subject name in OnDemand? HOT 5
- FileStorage Delete doesn't delete non-empty directories HOT 7
- Implement ARI HOT 2
- How to disable logs? HOT 1
- Panic on ZeroSSL API Issuer when no `Storage` is set HOT 3
- Looking for cause and solution to "config returned for certificate is not nil and points to different cache" error returned in cache.go HOT 3
- Allow Certmagic to generate 'next' private key to allow safe TLSA/DANE deployment and rollover HOT 5
- Use posix file advisory locks on supported platforms HOT 1
- Sometimes generating 33 ARI requests in a single second HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from certmagic.