bilelmoussaoui / oo7

James Bond went on a new mission as a Secret Service provider
https://bilelmoussaoui.github.io/oo7/oo7/
MIT License
57 stars 11 forks source link

Portal implementation allows concurrent writes that fail because of mtime check #52

Closed zecakeh closed 7 months ago

zecakeh commented 7 months ago

It just stumbled upon this in Fractal. I wrote a migration for the secrets we store, but when spawning 2 Item::delete(), we get portal::Error::TargetFileChanged(_) on one of them.

It seems like it is due to several tasks being able to read mtime in portal::Keyring::write(), because its in a RwLock. But as soon as the first task changes the file, portal::api::Keyring::dump() fails for subsequent tasks when checking if the mtime is still correct.

I believe this could be solved by using a Mutex instead for mtime. That way only one portal::api::Keyring::dump() can be called at a time, always with the latest mtime.

bilelmoussaoui commented 7 months ago

A PR with potentially a test case would be very appreciated :)