Commit 4694f5b
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
1 file changed
Lines changed: 5 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -107,8 +107,7 @@ inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init, | |||
| 107 | 107 | js_promise_hooks_[3].Reset(env()->isolate(), resolve); | |
| 108 | 108 | for (auto it = contexts_.begin(); it != contexts_.end(); it++) { | |
| 109 | 109 | if (it->IsEmpty()) { | |
| 110 | - it = contexts_.erase(it); | ||
| 111 | - it--; | ||
| 110 | + contexts_.erase(it--); | ||
| 112 | 111 | continue; | |
| 113 | 112 | } | |
| 114 | 113 | PersistentToLocal::Weak(env()->isolate(), *it) | |
@@ -251,12 +250,11 @@ inline void AsyncHooks::AddContext(v8::Local<v8::Context> ctx) { | |||
| 251 | 250 | inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) { | |
| 252 | 251 | v8::Isolate* isolate = env()->isolate(); | |
| 253 | 252 | 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()); | ||
| 254 | 257 | for (auto it = contexts_.begin(); it != contexts_.end(); it++) { | |
| 255 | - if (it->IsEmpty()) { | ||
| 256 | - it = contexts_.erase(it); | ||
| 257 | - it--; | ||
| 258 | - continue; | ||
| 259 | - } | ||
| 260 | 258 | v8::Local<v8::Context> saved_context = | |
| 261 | 259 | PersistentToLocal::Weak(isolate, *it); | |
| 262 | 260 | if (saved_context == ctx) { | |
0 commit comments