🐛 Fix lost-update race on team.features during concurrent file cr… (#9198)

* 🐛 Fix lost-update race on team.features during concurrent file creation

* 📚 Add CHANGES.md entry for team.features race condition fix (#9197)

CHANGES.md

@@ -73,6 +73,7 @@
 
 ### :bug: Bugs fixed
 
+- Fix lost-update race on `team.features` during concurrent file creation: two simultaneous create-file requests on the same team could both read the same features snapshot, compute different unions, and have the second `UPDATE` silently overwrite the first; the write is now preceded by a `SELECT … FOR UPDATE` inside the same transaction so every update sees the latest committed state [Github #9197](https://github.com/penpot/penpot/issues/9197)
 - Fix Alt/Option to draw shapes from center point (by @offreal) [Github #8361](https://github.com/penpot/penpot/pull/8361)
 - Add token name on broken token pill on sidebar [Taiga #13527](https://tree.taiga.io/project/penpot/issue/13527)
 - Fix tooltip activated when tab change [Taiga #13627](https://tree.taiga.io/project/penpot/issue/13627)

backend/src/app/rpc/commands/files_create.clj

@@ -112,22 +112,30 @@
                         ::quotes/profile-id profile-id
                         ::quotes/project-id project-id})
 
-    ;; FIXME: IMPORTANT: this code can have race conditions, because
-    ;; we have no locks for updating team so, creating two files
-    ;; concurrently can lead to lost team features updating
-    (when-let [features (-> features
-                            (set/difference (:features team))
-                            (set/difference cfeat/no-team-inheritable-features)
-                            (not-empty))]
-      (let [features (-> features
-                         (set/union (:features team))
-                         (set/difference cfeat/no-team-inheritable-features)
-                         (into-array))]
+    ;; Acquire a row-level lock on the team and re-read its features
+    ;; inside the same transaction before the read-modify-write below.
+    ;; Without the lock, two concurrent create-file calls on the same
+    ;; team can both observe the same team.features value, each
+    ;; compute a different union, and the second UPDATE silently
+    ;; overwrites the first (lost update under READ COMMITTED).
+    (let [team-features (-> (db/exec-one! conn
+                                          ["SELECT features FROM team WHERE id = ? FOR UPDATE"
+                                           team-id])
+                            :features
+                            (db/decode-pgarray #{}))]
+      (when-let [new-features (-> features
+                                  (set/difference team-features)
+                                  (set/difference cfeat/no-team-inheritable-features)
+                                  (not-empty))]
+        (let [features (-> new-features
+                           (set/union team-features)
+                           (set/difference cfeat/no-team-inheritable-features)
+                           (into-array))]
 
-        (db/update! conn :team
-                    {:features features}
-                    {:id (:id team)}
-                    {::db/return-keys false})))
+          (db/update! conn :team
+                      {:features features}
+                      {:id team-id}
+                      {::db/return-keys false}))))
 
     (-> (create-file cfg params)
         (vary-meta assoc ::audit/props {:team-id team-id}))))