Workspace store deadlock protection#9132
Conversation
touilleMan
force-pushed
the
workspace-store-deadlock-protection
branch
6 times, most recently
from
0debfb6 to
563eb26
Compare
|
|
||
| let outcome = cache_guard.lock_update_manifests.take(entry_id); | ||
| match outcome { | ||
| enum LockForUpdateOutcome { |
There was a problem hiding this comment.
Isn't that the same struct as in file_updater.rs ?
There was a problem hiding this comment.
Yes, but I don't think it is worth it to factorize it since this is purely implementation detail of for_update_* code (that's why it is defined within the function body).
Basically factorize just a type would create an artificial relationship between two unrelated pieces of code that use it, which typically cause issue when modifying one piece but not the other (on top of adding complexity when reading the code since the type definition end up in another file).
However you're right on the fact both for_update_* functions have very similar implementations, so we should be able to factorize the entire implementation. I will look into this in my next PR (see #9153) that will modify part of this code anyway.
touilleMan
force-pushed
the
workspace-store-deadlock-protection
branch
from
563eb26 to
7f83bbe
Compare
…ock detector \o/)
touilleMan
force-pushed
the
workspace-store-deadlock-protection
branch
from
7f83bbe to
3233fd5
Compare
| // parent (i.e. `for_write`) or the grand-parent (i.e. | ||
| // `CertificateOps::add_certificates_batch`) which are going to poll this | ||
| // future directly, so the references' lifetimes *are* long enough. | ||
| // TODO: Remove this once async closure are available |
There was a problem hiding this comment.
FYI: Will be available in 1.85
3 participants
WorkspaceStore's cache & storage (i.e. local db) internals