Skip to content

Commit

Permalink
Fix a race condition that leads to a CouchDB conflict (#4158)
Browse files Browse the repository at this point in the history
If Alice and Bob shares a file within a Cozy to Cozy sharing. The file
is present on both cozy instances, and the two users make a change on
this file concurrently. For example, Alice adds the file to a photo
album, and Bob renames it. A race condition may happen when Bob's
instance sends its update to Alice's instance. The CouchDB document will
then been saved in CouchDB with a conflict (two leaf revisions).

```
+----------------------------------+----------------------------------+
| Goroutine A                      | Goroutine B                      |
| Add the file to an album         | Share-replicate sends the rename |
+----------------------------------+----------------------------------+
|                                  | 1. Load the file from CouchDB    |
| 2. Load the file from CouchDB    |                                  |
| 3. Get the VFS lock              |                                  |
| 4. Save the file with rev 2-aaa  |                                  |
| 5. Release the VFS lock          |                                  |
|                                  | 6. Get the VFS lock              |
|                                  | 7. Save the file with rev 2-bbb  |
|                                  | 8. Release the VFS lock          |
+----------------------------------+----------------------------------+
```

In general, it is not possible to save a document with a revision 2-bbb,
when the document is already at revision 2-aaa: CouchDB responds with a
409 Conflict. But, the share-track is doing a bulk update with the
option new_edit: false. This is done because the replication process
needs to force the same revision on Alice's Cozy that it is on Bob's
Cozy. But, it also enable to create a conflict in CouchDB.

The fix is to reload the file between the steps 6 and 7 (when the VFS
lock has been acquired), and check that the revision has not changed. It
would be nice to load the document when the VFS lock is acquired, but it
is not possible currently because of how the code is organized.
  • Loading branch information
nono authored Oct 11, 2023
2 parents 1753348 + 1b110e2 commit afe48f0
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions model/sharing/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,22 @@ func (s *sharingIndexer) setDirOrFileRevisions(oldDocDir *vfs.DirDoc, oldDocFile
}

func (s *sharingIndexer) bulkForceUpdateDoc(olddoc, doc *vfs.FileDoc) error {
// XXX We need to check that the file has not been updated between it has
// been loaded and the call to BulkUpdateDocs, as the VFS lock has been
// acquired after the file has been loaded, and CouchDB can create a
// conflict in the database if it happens (because of the new_edit: false
// option).
if olddoc != nil {
var current couchdb.JSONDoc
err := couchdb.GetDoc(s.db, consts.Files, doc.ID(), &current)
if err != nil {
return err
}
if current.Rev() != olddoc.Rev() {
return ErrInternalServerError
}
}

docs := make([]map[string]interface{}, 1)
docs[0] = map[string]interface{}{
"type": doc.Type,
Expand Down

0 comments on commit afe48f0

Please sign in to comment.