[RFC PATCH 12/13] review_tui: write git tracking commit before updating DB

Adrian Neftali Sanchez <[email protected]>
Newsgroups org.kernel.linux.tools
Message-ID <[email protected]>
The background rescan_branches worker reads the git tracking commit and
writes the status it finds back to the DB.  Both action_review and
action_waiting previously wrote to the DB first and updated the git
tracking commit second; if the rescan worker woke up in that window it
would overwrite the DB row with the stale status from the old commit.

On Linux this window is rarely hit because epoll wakes the event loop
only when I/O is ready.  Windows uses IocpProactor, which processes
completion callbacks eagerly and causes the background rescan task to
be scheduled between the two writes reliably, making the race
consistently reproducible in the test suite.

Fix both actions so the git tracking commit is always written first.
The rescan then reads the already-updated commit and any DB write it
performs is idempotent with what the action itself is about to write.

Signed-off-by: Adrian Neftali Sanchez <[email protected]>
---
 src/b4/review_tui/_tracking_app.py | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index a5a9389..d52a49b 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -1245,16 +1245,20 @@ class TrackingApp(CheckRunnerMixin, App[Optional[str]]):
                             conn, change_id, 'reviewing', revision=revision
                         )
                 elif status in ('waiting', 'accepted'):
-                    # Bring back to reviewing on re-entry
-                    if conn:
-                        b4.review.tracking.update_series_status(
-                            conn, change_id, 'reviewing', revision=revision
-                        )
+                    # Bring back to reviewing on re-entry.
+                    # Update git FIRST so that any concurrent rescan reads the
+                    # new status from the tracking commit before we commit it
+                    # to the DB; this prevents the rescan from overwriting the
+                    # DB with the stale status on the next wake-up.
                     topdir = b4.git_get_toplevel()
                     if topdir:
                         b4.review.update_tracking_status(
                             topdir, branch_name, 'reviewing'
                         )
+                    if conn:
+                        b4.review.tracking.update_series_status(
+                            conn, change_id, 'reviewing', revision=revision
+                        )
                 # Clear the followup badge — user is about to read this series
                 if conn and self._identifier and isinstance(revision, int):
                     b4.review.tracking.mark_all_messages_seen(conn, change_id, revision)
@@ -4078,6 +4082,12 @@ class TrackingApp(CheckRunnerMixin, App[Optional[str]]):
             return
         change_id = self._selected_series.get('change_id', '')
         revision = self._selected_series.get('revision')
+        # Update git FIRST so that any concurrent rescan reads the new status
+        # from the tracking commit before we commit it to the DB.
+        topdir = b4.git_get_toplevel()
+        if topdir and status != 'new':
+            branch_name = f'b4/review/{change_id}'
+            b4.review.update_tracking_status(topdir, branch_name, 'waiting')
         try:
             conn = b4.review.tracking.get_db(self._identifier)
             b4.review.tracking.update_series_status(
@@ -4087,10 +4097,6 @@ class TrackingApp(CheckRunnerMixin, App[Optional[str]]):
         except Exception as ex:
             self.notify(f'Error: {ex}', severity='error')
             return
-        topdir = b4.git_get_toplevel()
-        if topdir and status != 'new':
-            branch_name = f'b4/review/{change_id}'
-            b4.review.update_tracking_status(topdir, branch_name, 'waiting')
         self.notify('Series moved to waiting')
         self._focus_change_id = change_id
         self._invalidate_caches(change_id)
-- 
2.45.0.windows.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.