All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Neftali Sanchez <lutgaru@gmail.com>
To: tools@linux.kernel.org
Cc: konstantin@linuxfoundation.org,
	Adrian Neftali Sanchez <lutgaru@gmail.com>
Subject: [RFC PATCH 12/13] review_tui: write git tracking commit before updating DB
Date: Sat, 25 Apr 2026 12:49:40 -0700	[thread overview]
Message-ID: <20260425194943.1499-13-lutgaru@gmail.com> (raw)
In-Reply-To: <20260425194943.1499-1-lutgaru@gmail.com>

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 <lutgaru@gmail.com>
---
 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


  parent reply	other threads:[~2026-04-25 19:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-25 19:49 [RFC PATCH 00/13] b4: add native Windows support Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 01/13] tests: specify UTF-8 encoding when opening files in text mode Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 02/13] tests: use os.path.join/normpath for portable path assertions Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 03/13] b4: add cross-platform username resolution in _setup_user_config Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 04/13] b4: resolve platform-native data and cache directories Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 05/13] b4: normalise git-reported absolute paths with os.path.normpath Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 06/13] b4: select platform-appropriate default pager Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 07/13] ez: use portable uid/gid accessors in write_to_tar Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 08/13] tui: The _suspend_to_shell function assumes a POSIX environment, relying on $SHELL and shell-specific arguments (e.g., --rcfile) that are invalid on Windows Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 09/13] tests: replace NamedTemporaryFile with tmp_path in patatt fixture Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 10/13] pyproject: add Windows operating-system classifier Adrian Neftali Sanchez
2026-04-25 19:49 ` [RFC PATCH 11/13] ci: add ci.ps1 for running the quality-gate pipeline on Windows Adrian Neftali Sanchez
2026-04-25 19:49 ` Adrian Neftali Sanchez [this message]
2026-04-25 19:49 ` [RFC PATCH 13/13] review_tui: guard against unmounted diff-viewer In certain asynchronous contexts or during rapid UI transitions, _show_content may be invoked before the diff-viewer widget has been fully mounted in the DOM. This is particularly reproducible in headless test environments and on platforms with slower console I/O initialization Adrian Neftali Sanchez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260425194943.1499-13-lutgaru@gmail.com \
    --to=lutgaru@gmail.com \
    --cc=konstantin@linuxfoundation.org \
    --cc=tools@linux.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.