Bug 1875859 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

The [comparator as such](https://searchfox.org/mozilla-central/rev/3c72de9280ec57dc55c24886c6334d9e340500e8/netwerk/cache2/CacheStorageService.cpp#996-998) looks pretty trivial. But is there a chance that `mozilla::net::DoUpdateExpirationTime` can happen while sorting? AFAICS `CacheEntry::GetExpirationTime` asserts to be on a certain thread (and is used here by the comparator), but `CacheEntry::SetExpirationTime` does not, not sure if that means it is supposed to be called from other threads, too? If so we would definitely need to make a copy of the array before sorting or otherwise add some mutex that encloses the entire sorting.

:jesup, wdyt?
The [comparator as such](https://searchfox.org/mozilla-central/rev/3c72de9280ec57dc55c24886c6334d9e340500e8/netwerk/cache2/CacheStorageService.cpp#996-998) looks pretty trivial. But is there a chance that `mozilla::net::DoUpdateExpirationTime` can happen while sorting? AFAICS `CacheEntry::GetExpirationTime` asserts to be on a certain thread (and is used here by the comparator), but `CacheEntry::SetExpirationTime` does not, not sure if that means it is supposed to be called from other threads, too? If so we would definitely need to make a copy of the array behind a mutex before sorting or otherwise add some mutex that encloses the entire sorting.

:jesup, wdyt?
The [comparator as such](https://searchfox.org/mozilla-central/rev/3c72de9280ec57dc55c24886c6334d9e340500e8/netwerk/cache2/CacheStorageService.cpp#996-998) looks pretty trivial. But is there a chance that `mozilla::net::DoUpdateExpirationTime` can happen while sorting? AFAICS `CacheEntry::GetExpirationTime` asserts to be on a certain thread (and is used here by the comparator), but `CacheEntry::SetExpirationTime` does not, not sure if that means it is supposed to be called from other threads, too? If so we would definitely need to make a copy of the array behind a mutex before sorting or otherwise add some mutex that encloses the entire sorting.

:jesup, wdyt?

Edit: There is [also this scary comment](https://searchfox.org/mozilla-central/rev/3c72de9280ec57dc55c24886c6334d9e340500e8/netwerk/cache2/CacheEntry.cpp#1165-1166): `// Aligned assignment, thus atomic.` - that hints indeed to some concurrency here? It is worth mentioning that sorting in general can only work reliably if the comparator results for given a and b do not change while sorting and `std::sort` in particular is not doing any additional bounds checks if things go wrong.

Back to Bug 1875859 Comment 5