[Autofill sync] Race condition when both PDM and sync write to the DB |
||||
Issue descriptionThis is a problem causing flakes in an integration tests (before I've rewritten it to avoid it). There could be multiple instances of the same concept, I've hit this particular one: If a new profile (address) is added to wallet data when >=2 syncing clients are on, we can hit a race condition: 1) Both clients get the new address and independently create new metadata entity for it (each with a different use_date()) and commit it 2) On each client, PDM converts the new address to a local address. 3) On the client with lower use_date() in particular order: a) The remote update comes in and sync updates the metadata with a newer use_date. b) The local address conversion is done and PDM updates the metadata with has_converted=true. In either case, the updates that comes as last overwrites the previous update and we end up in an inconsistent state.
,
Dec 3
If checked other potential of such race condition: there are but the chances are low and the effect is also tolerable. Some writes by PDM react to a user action: i) incrementing the use stats when a card / address gets used and ii) updating billing_address_id when the user edits that in the UI. It is not very likely that the users does change metadata on the second device while using / editing a card on the first device.
,
Dec 3
Lastly, billing_address_id sometimes gets updated automatically (notably when garbage collecting unused addresses on browser startup). I guess there could be a race condition between updating the billing_address_id and doing some random remote update (e.g. incrementing use stats or changing the billing address in the UI). I was wrong in the analysis in the original report, the updates from sync happen atomically, i.e. the sync bridge cannot overwrite changes made by the PDM. It is still correct that the PDM can overwrite changes made by the sync bridge. Thus, it can happen the the user updates billing address on device 1 from address A that has not been used for long to address B used more recently while device 2 is off. When starting up device 2, the address A gets wiped by garbage collection and PDM clears billing_address_id in the metadata, accidentally overwriting the update made by device 2. Eventually, the empty value set by device 1 wins even though the user has clearly set it to address B. Need to think if this is a real and severe enough problem...
,
Dec 12
Thanks for the thorough report. Even though this is less than ideal, it's not too much of big deal for now. The reason is that the only feature that used billing address was Payment Request but they are not using Chrome cards anymore. We should still try to fix it, or at least document it in case it gets used again in the future.
,
Dec 12
,
Dec 12
,
Jan 8
It goes a bit beyond the scope of this bug but I've encountered a similar race condition in autofill_profile in integration tests. Renaming the bug to reflect that.
1) Two clients simultaneously get a new server profile.
2) Client A converts it to local profile P and syncs the new profile up
3) Client B is converting the new server profile to a local profile P' in the PDM (P is equivalent to P', only has a different guid). Client B posts a task to the DB thread to add P' into the DB.
4) Client B receives the converted profile P from sync and writes it into the DB
5) The posted task executes on client B and the profile P' gets written to the DB as well.
As a result client B has two equivalent local profiles {P,P'}. They eventually get de-duped but this behavior is still suboptimal. This leads to a rare race condition in an integration test, see Issue 917498.
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jkrcal@chromium.org
, Dec 3