From 1b110e2b280fcaf4c0f2afb9e518dc055daff3ee Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Wed, 11 Oct 2023 17:06:01 +0200 Subject: [PATCH] Fix a race condition that leads to a CouchDB conflict 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. --- model/sharing/indexer.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/model/sharing/indexer.go b/model/sharing/indexer.go index 3abd7f91445..c33f2183a8e 100644 --- a/model/sharing/indexer.go +++ b/model/sharing/indexer.go @@ -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(), ¤t) + 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,