Make CollectSubgridItemsForAxis iteration more hygenic
Categories
(Core :: Layout: Grid, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox123 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
Details
Attachments
(1 file)
In CollectSubgridItemsForAxis's helper CollectSubgridForAxis, we iterate over one nsTArray called mGridItems, and we append to another one called aResult. But in fact those are the same array, since the caller passes in mGridItems for the outparam. This can result in the internal array being reallocated out from under us, which could make us crash if we use stale data after reallocation happens.
As it turns out, we don't use stale data and don't crash right now, because we get lucky:
(a) we don't use the current array element that we were referencing after we append.
(b) the ranged-based iteration rediscovers the (up-to-date) "end" iterator on each iteration of the loop.
(As far as I can tell, those are the only avenues where we might [but don't] use stale data.)
Anyway -- it'd be better and more future-proof if we set things up such that we're appending to a temporary array while we iterate, and then append that temporary array to mGridItems once we're done. Then we'd still be fine if either the aforementioned (a) or (b) factors change in the future (i.e. if we start using data from the current array element even after we append to the array).
|
Assignee | |
Comment 1•2 years ago
|
||
This patch doesn't change behavior.
Before this patch, we're appending to the array that we're iterating over,
which is risky, though it happens to be safe right now since we don't use
any of the newly-invalid-data after the array is mutated.
This patch just uses a less risky approach (appending to a temporary array)
to avoid this fragility.
|
||
Comment 3•2 years ago
|
||
| bugherder | ||
Description
•