Sync connections: nothing stops one connection running twice at once, and the DAV sync then duplicates contacts #586

New issue

Closed

opened 2026-10-01 10:47:51 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:51 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d and postulo-dav at 97abc9779, looking for what serialises two runs of one sync connection and simulating the overlap against the plugin's in-memory server.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/views.py:452-456 (ConnectionSyncNowView.post) and notifications/management/commands/send_due_reminders.py:189-198 (the pass under only_one_pass); plugins/syncing.py:70-96 and core/scheduler.py:91-98 have not moved.

sync_connection takes no lock. The scheduler's lease covers one scheduler pass against another, and Sync now calls sync_connection directly, so a press of Sync now during a pass, or from two tabs, runs the same connection twice at once. A sync plugin decides what to push and what to adopt from the links it read at the start, so two runs that overlap each do the other's work again.

Where: plugins/syncing.py:70-96 (sync_connection), plugins/views.py:451-455 (ConnectionSyncNowView.post), core/scheduler.py:91-98 (only_one_pass) as used in notifications/management/commands/send_due_reminders.py:187-196, config/settings/base.py:140 (ATOMIC_REQUESTS). In postulo-dav: src/postulo_dav/sync.py:437-457 (a new contact is pushed to a random uuid4() address), :527-550 (every unlinked card is adopted), :576-577 (an interview's first push).
What happens: Sync now runs inside the request's transaction, so the links it writes are invisible to any other process until the response is sent. A scheduler pass that reaches the same connection in that time sees each new contact as unpushed and each card the request has just written as a stranger's. It pushes the contact again, under a second random address that If-None-Match: * cannot refuse, and adopts the first card as a new contact. Simulated in one process by running a second sync between the first one's PUT and its SyncLink.bind: one contact ended as two cards, and after the following run as three contacts of the same name. For interviews the address is <uid>.ics, so the second run meets a 412 that nothing catches at sync.py:577 and fails. A server that enforces unique UIDs refuses the second card and the run fails there instead. The double-submit guard in static/js/app.js covers one form on one page, not a second tab or the scheduler.
Why it matters: duplicate contacts here and duplicate cards in the address book, which the person has to find and remove by hand on both sides; with any future sync plugin the same overlap is open.
Fix: hold a lease per connection around the plugin call in sync_connection, in the shape of only_one_pass, and let the run that does not get it return a report that says a sync is already running (a translated sentence; Sync now shows it as information, not as a failure). The lease has to be visible outside the request's transaction: the default cache is the database cache on the same connection, so a cache.add made inside an ATOMIC_REQUESTS request is not committed until the request ends. Either take it on a path that commits at once, or run Sync now as an errand outside the request. In postulo-dav, deriving a new card's address from its UID would let If-None-Match: * refuse a second push; related to postulo/postulo-dav#7, which pairs by UID and makes a 412 on one item a note.

Done when

tests/test_syncs.py has a test with a sync plugin whose sync calls sync_connection on its own connection: the inner call returns the "already running" report without calling the plugin, and the lease is gone afterwards, also when the plugin raises.
A test that Sync now on a connection whose lease is held answers with the information message and does not run the plugin.

Found on 2026-09-29 in a read-only review of postulo at 95348908d and postulo-dav at 97abc9779, looking for what serialises two runs of one sync connection and simulating the overlap against the plugin's in-memory server. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/views.py:452-456` (`ConnectionSyncNowView.post`) and `notifications/management/commands/send_due_reminders.py:189-198` (the pass under `only_one_pass`); `plugins/syncing.py:70-96` and `core/scheduler.py:91-98` have not moved. `sync_connection` takes no lock. The scheduler's lease covers one scheduler pass against another, and *Sync now* calls `sync_connection` directly, so a press of *Sync now* during a pass, or from two tabs, runs the same connection twice at once. A sync plugin decides what to push and what to adopt from the links it read at the start, so two runs that overlap each do the other's work again. - **Where:** `plugins/syncing.py:70-96` (`sync_connection`), `plugins/views.py:451-455` (`ConnectionSyncNowView.post`), `core/scheduler.py:91-98` (`only_one_pass`) as used in `notifications/management/commands/send_due_reminders.py:187-196`, `config/settings/base.py:140` (`ATOMIC_REQUESTS`). In postulo-dav: `src/postulo_dav/sync.py:437-457` (a new contact is pushed to a random `uuid4()` address), `:527-550` (every unlinked card is adopted), `:576-577` (an interview's first push). - **What happens:** *Sync now* runs inside the request's transaction, so the links it writes are invisible to any other process until the response is sent. A scheduler pass that reaches the same connection in that time sees each new contact as unpushed and each card the request has just written as a stranger's. It pushes the contact again, under a second random address that `If-None-Match: *` cannot refuse, and adopts the first card as a new contact. Simulated in one process by running a second sync between the first one's `PUT` and its `SyncLink.bind`: one contact ended as two cards, and after the following run as three contacts of the same name. For interviews the address is `<uid>.ics`, so the second run meets a 412 that nothing catches at `sync.py:577` and fails. A server that enforces unique UIDs refuses the second card and the run fails there instead. The double-submit guard in `static/js/app.js` covers one form on one page, not a second tab or the scheduler. - **Why it matters:** duplicate contacts here and duplicate cards in the address book, which the person has to find and remove by hand on both sides; with any future sync plugin the same overlap is open. - **Fix:** hold a lease per connection around the plugin call in `sync_connection`, in the shape of `only_one_pass`, and let the run that does not get it return a report that says a sync is already running (a translated sentence; *Sync now* shows it as information, not as a failure). The lease has to be visible outside the request's transaction: the default cache is the database cache on the same connection, so a `cache.add` made inside an `ATOMIC_REQUESTS` request is not committed until the request ends. Either take it on a path that commits at once, or run *Sync now* as an errand outside the request. In postulo-dav, deriving a new card's address from its UID would let `If-None-Match: *` refuse a second push; related to postulo/postulo-dav#7, which pairs by UID and makes a 412 on one item a note. ## Done when - `tests/test_syncs.py` has a test with a sync plugin whose `sync` calls `sync_connection` on its own connection: the inner call returns the "already running" report without calling the plugin, and the lease is gone afterwards, also when the plugin raises. - A test that *Sync now* on a connection whose lease is held answers with the information message and does not run the plugin.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:51 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:47:51 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 02:35:50 +00:00

Run one sync connection at a time

tiagoagueda closed this issue

2026-10-04 02:35:50 +00:00