Skip to content

Commit 4364926

Browse files
Deniz Evrencifacebook-github-bot
authored andcommitted
Prevent accessing invalidated iterators in TDigest::merge
Summary: According to the standard, `std::vector::insert` invalidates the end iterator (and any iterators past the point of insertion). This seems to be benign in practice but MSVC's iterator debug assertions catch it. We can preserve the spirit of the original code by replacing the iterators with pointers or indexes. I chose the latter. Differential Revision: D47218022 fbshipit-source-id: 96808b20a9483b0ce0a4939edb3de48f48ef4f22
1 parent 640749c commit 4364926

File tree

1 file changed

+8
-5
lines changed

1 file changed

+8
-5
lines changed

third-party/folly/src/folly/stats/TDigest.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ TDigest TDigest::merge(Range<const TDigest*> digests) {
222222
std::vector<Centroid> centroids;
223223
centroids.reserve(nCentroids);
224224

225-
std::vector<std::vector<Centroid>::iterator> starts;
225+
std::vector<size_t> starts;
226226
starts.reserve(digests.size());
227227

228228
double count = 0;
@@ -233,7 +233,7 @@ TDigest TDigest::merge(Range<const TDigest*> digests) {
233233
double max = -std::numeric_limits<double>::infinity();
234234

235235
for (const auto& digest : digests) {
236-
starts.push_back(centroids.end());
236+
starts.push_back(centroids.size());
237237
double curCount = digest.count();
238238
if (curCount > 0) {
239239
DCHECK(!std::isnan(digest.min_));
@@ -262,9 +262,12 @@ TDigest TDigest::merge(Range<const TDigest*> digests) {
262262
// digestsPerBlock big). In that case, merge to end. Otherwise, merge to
263263
// the end of that block.
264264
auto last = (i + (digestsPerBlock * 2) < startsSize)
265-
? *(starts.begin() + i + 2 * digestsPerBlock)
266-
: centroids.end();
267-
std::inplace_merge(first, middle, last);
265+
? starts[i + 2 * digestsPerBlock]
266+
: centroids.size();
267+
std::inplace_merge(
268+
centroids.begin() + first,
269+
centroids.begin() + middle,
270+
centroids.begin() + last);
268271
}
269272
}
270273
}

0 commit comments

Comments
 (0)