From 8f3ce94bd04ea3f53cce4a42c7d0656ba171b083 Mon Sep 17 00:00:00 2001 From: Gabriel Bota Date: Fri, 15 Apr 2022 16:22:08 +0200 Subject: [PATCH 1/2] 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. --- src/env-inl.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 21d365c04ba3c3..7cac02e70ffa54 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -107,8 +107,7 @@ inline void AsyncHooks::SetJSPromiseHooks(v8::Local init, js_promise_hooks_[3].Reset(env()->isolate(), resolve); for (auto it = contexts_.begin(); it != contexts_.end(); it++) { if (it->IsEmpty()) { - it = contexts_.erase(it); - it--; + contexts_.erase(it--); continue; } PersistentToLocal::Weak(env()->isolate(), *it) @@ -251,12 +250,12 @@ inline void AsyncHooks::AddContext(v8::Local ctx) { inline void AsyncHooks::RemoveContext(v8::Local ctx) { v8::Isolate* isolate = env()->isolate(); v8::HandleScope handle_scope(isolate); + contexts_.erase(std::remove_if(contexts_.begin(), contexts_.end(), + [&](auto&& el) { + return el.IsEmpty(); + }), + contexts_.end()); for (auto it = contexts_.begin(); it != contexts_.end(); it++) { - if (it->IsEmpty()) { - it = contexts_.erase(it); - it--; - continue; - } v8::Local saved_context = PersistentToLocal::Weak(isolate, *it); if (saved_context == ctx) { From 84dd5694e43a2b44f287c62a329d0fed6fe8ecc0 Mon Sep 17 00:00:00 2001 From: Gabriel Bota Date: Fri, 15 Apr 2022 16:36:41 +0200 Subject: [PATCH 2/2] fixup! async_hooks: avoid decrementing iterator after erase --- src/env-inl.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 7cac02e70ffa54..46feb9bfa4ca93 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -250,10 +250,9 @@ inline void AsyncHooks::AddContext(v8::Local ctx) { inline void AsyncHooks::RemoveContext(v8::Local ctx) { v8::Isolate* isolate = env()->isolate(); v8::HandleScope handle_scope(isolate); - contexts_.erase(std::remove_if(contexts_.begin(), contexts_.end(), - [&](auto&& el) { - return el.IsEmpty(); - }), + contexts_.erase(std::remove_if(contexts_.begin(), + contexts_.end(), + [&](auto&& el) { return el.IsEmpty(); }), contexts_.end()); for (auto it = contexts_.begin(); it != contexts_.end(); it++) { v8::Local saved_context =