Skip to content

Commit 4694f5b

Browse files
dygabotargos
authored andcommitted
async_hooks: avoid decrementing iterator after erase
decrementing an iterator returned by `std::vector::erase` may have undefined behaviour and should not be used. Decrementing `end()` on an empty container is undefined and `.erase()` could leave the container empty. Instead, by calling `vec.erase(it--)` we decrement the valid iterator before the erase operation but after being passed to the erase method. In case of `AsyncHooks::RemoveContext` perform the cleanup of empty contexts upfront using `std::remove_if` because the iteration gets interrupted as soon as the context to be removed has been found. PR-URL: #42749 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent 93fd77a commit 4694f5b

File tree

1 file changed

+5
-7
lines changed

1 file changed

+5
-7
lines changed

src/env-inl.h

+5-7
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,7 @@ inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
107107
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
108108
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
109109
if (it->IsEmpty()) {
110-
it = contexts_.erase(it);
111-
it--;
110+
contexts_.erase(it--);
112111
continue;
113112
}
114113
PersistentToLocal::Weak(env()->isolate(), *it)
@@ -251,12 +250,11 @@ inline void AsyncHooks::AddContext(v8::Local<v8::Context> ctx) {
251250
inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) {
252251
v8::Isolate* isolate = env()->isolate();
253252
v8::HandleScope handle_scope(isolate);
253+
contexts_.erase(std::remove_if(contexts_.begin(),
254+
contexts_.end(),
255+
[&](auto&& el) { return el.IsEmpty(); }),
256+
contexts_.end());
254257
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
255-
if (it->IsEmpty()) {
256-
it = contexts_.erase(it);
257-
it--;
258-
continue;
259-
}
260258
v8::Local<v8::Context> saved_context =
261259
PersistentToLocal::Weak(isolate, *it);
262260
if (saved_context == ctx) {

0 commit comments

Comments
 (0)