2043202 - TrackingDBService._flushLiveLogs early-returns concurrent callers, causing stale reads

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect, P2)

Description

[Petru-Mugurel Lingurar [:petru]]

Attachment: TrackingProtectionsDashboardMockup.png

Fenix needs to show in it's new tracking protection dashboard the details about the running count of all trackers blocked, the first record of that and how many trackers have been blocked this week. Screenshot attached of the proposed UI.

While working on integrating the sumAllEvents, getEventsByDateRange and getEarliestRecordedDate calls from TrackingDBService.sys.mjs in Fenix I saw that calingl getEventsByDateRange and sumAllEvents back-to-back will ensure only the first query that enters Gecko will returns fresh data while the other will lag by one event cycle and will only catch up when traffic stops.
Swapping the calls order inverts which result is stale, confirming there is a race.

Upon more investigation it seems like when _flushLiveLogs is invoked while another flush is in progress, the _flushingLiveLogs guard returns immediately from here without awaiting the in-flight flush.
The concurrent caller then runs its SQL against the un-flushed DB and gets a stale snapshot.

Comment 1

[Tim Huang[:timhuang]]

Thanks for the analysis. I think we need to find a way to ensure we have up-to-date data for the second call. We can queue the task of _flushLiveLogs and only resolve the tasks once the flushing is finished, instead of returning it right away. I will work on the patch.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Thanks for looking into this!
We have different scenarios in the app, some that would make only one call - total trackers blocked shown in the trackers pill on home, some that would make all 3 calls - the dashboard.
Think your idea would support this nicely.

Comment 3

[Tim Huang[:timhuang]]

Attachment: Bug 2043202 - Use a shared promise to ensure concurrent call to _flushLiveLogs(). r?#anti-tracking!

This patch changes to use a shared promise in _flushLiveLogs to ensure
concurrent calls to this function will all wait until the data is
flushed. This patch fixes one issue that the second call into the
tracking DB service APIs will return stale data.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a64b828f0cfa
https://hg.mozilla.org/integration/autoland/rev/a31b856a54d0
Use a shared promise to ensure concurrent call to _flushLiveLogs(). r=emz

Comment 5

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/a31b856a54d0

Comment 6

[BMO Github Automation]

Authored by https://github.com/artines1
https://github.com/mozilla/enterprise-firefox/commit/a64b828f0cfa5a011507a615bdd1f850ae02eef1
[enterprise-main] Bug 2043202 - Use a shared promise to ensure concurrent call to _flushLiveLogs(). r=emz