diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-02-16 13:54:22 +0100 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-02-20 16:49:02 +0100 |
commit | 49a2e406329269af2bf55c0ad51560bdf1f14b7f (patch) | |
tree | 23b4e1a6e302140ca1b1bad863f68cadf5c688dd /src | |
parent | fab06681236994c3ac77a1f259373fb9731ed6f7 (diff) | |
download | android-node-v8-49a2e406329269af2bf55c0ad51560bdf1f14b7f.tar.gz android-node-v8-49a2e406329269af2bf55c0ad51560bdf1f14b7f.tar.bz2 android-node-v8-49a2e406329269af2bf55c0ad51560bdf1f14b7f.zip |
src: move req_wrap_queue to base class of ReqWrap
Introduce a second base class for `ReqWrap` that does not
depend on a template parameter and move the `req_wrap_queue_`
field to it.
This addresses undefined behaviour that occurs when casting
to `ReqWrap<uv_req_t>` in the `ReqWrap` constructor.
Refs: https://github.com/nodejs/node/issues/26131
PR-URL: https://github.com/nodejs/node/pull/26148
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/env.cc | 2 | ||||
-rw-r--r-- | src/env.h | 3 | ||||
-rw-r--r-- | src/node_postmortem_metadata.cc | 8 | ||||
-rw-r--r-- | src/node_process_methods.cc | 3 | ||||
-rw-r--r-- | src/req_wrap-inl.h | 17 | ||||
-rw-r--r-- | src/req_wrap.h | 24 |
6 files changed, 39 insertions, 18 deletions
diff --git a/src/env.cc b/src/env.cc index 63c5d65001..d882838ed0 100644 --- a/src/env.cc +++ b/src/env.cc @@ -420,7 +420,7 @@ void Environment::RegisterHandleCleanups() { } void Environment::CleanupHandles() { - for (ReqWrap<uv_req_t>* request : req_wrap_queue_) + for (ReqWrapBase* request : req_wrap_queue_) request->Cancel(); for (HandleWrap* handle : handle_wrap_queue_) @@ -873,8 +873,7 @@ class Environment { #endif typedef ListHead<HandleWrap, &HandleWrap::handle_wrap_queue_> HandleWrapQueue; - typedef ListHead<ReqWrap<uv_req_t>, &ReqWrap<uv_req_t>::req_wrap_queue_> - ReqWrapQueue; + typedef ListHead<ReqWrapBase, &ReqWrapBase::req_wrap_queue_> ReqWrapQueue; inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; } inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; } diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc index 527bfb623e..05800f79b0 100644 --- a/src/node_postmortem_metadata.cc +++ b/src/node_postmortem_metadata.cc @@ -28,15 +28,14 @@ V(Environment_HandleWrapQueue, head_, ListNode_HandleWrap, \ Environment::HandleWrapQueue::head_) \ V(ListNode_HandleWrap, next_, uintptr_t, ListNode<HandleWrap>::next_) \ - V(ReqWrap, req_wrap_queue_, ListNode_ReqWrapQueue, \ - ReqWrap<uv_req_t>::req_wrap_queue_) \ V(Environment_ReqWrapQueue, head_, ListNode_ReqWrapQueue, \ Environment::ReqWrapQueue::head_) \ - V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrap<uv_req_t>>::next_) + V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrapBase>::next_) extern "C" { int nodedbg_const_ContextEmbedderIndex__kEnvironment__int; uintptr_t nodedbg_offset_ExternalString__data__uintptr_t; +uintptr_t nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue; #define V(Class, Member, Type, Accessor) \ NODE_EXTERN uintptr_t NODEDBG_OFFSET(Class, Member, Type); @@ -51,6 +50,9 @@ int GenDebugSymbols() { ContextEmbedderIndex::kEnvironment; nodedbg_offset_ExternalString__data__uintptr_t = NODE_OFF_EXTSTR_DATA; + nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue = + OffsetOf<ListNode<ReqWrapBase>, ReqWrap<uv_req_t>>( + &ReqWrap<uv_req_t>::req_wrap_queue_); #define V(Class, Member, Type, Accessor) \ NODEDBG_OFFSET(Class, Member, Type) = OffsetOf(&Accessor); diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 80027c26eb..ca8a435805 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -252,7 +252,8 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) { Environment* env = Environment::GetCurrent(args); std::vector<Local<Value>> request_v; - for (auto w : *env->req_wrap_queue()) { + for (ReqWrapBase* req_wrap : *env->req_wrap_queue()) { + AsyncWrap* w = req_wrap->GetAsyncWrap(); if (w->persistent().IsEmpty()) continue; request_v.push_back(w->GetOwner()); diff --git a/src/req_wrap-inl.h b/src/req_wrap-inl.h index 4f9da1c4f3..5fb9654148 100644 --- a/src/req_wrap-inl.h +++ b/src/req_wrap-inl.h @@ -11,16 +11,16 @@ namespace node { +ReqWrapBase::ReqWrapBase(Environment* env) { + env->req_wrap_queue()->PushBack(this); +} + template <typename T> ReqWrap<T>::ReqWrap(Environment* env, v8::Local<v8::Object> object, AsyncWrap::ProviderType provider) - : AsyncWrap(env, object, provider) { - - // FIXME(bnoordhuis) The fact that a reinterpret_cast is needed is - // arguably a good indicator that there should be more than one queue. - env->req_wrap_queue()->PushBack(reinterpret_cast<ReqWrap<uv_req_t>*>(this)); - + : AsyncWrap(env, object, provider), + ReqWrapBase(env) { Reset(); } @@ -51,6 +51,11 @@ void ReqWrap<T>::Cancel() { uv_cancel(reinterpret_cast<uv_req_t*>(&req_)); } +template <typename T> +AsyncWrap* ReqWrap<T>::GetAsyncWrap() { + return this; +} + // Below is dark template magic designed to invoke libuv functions that // initialize uv_req_t instances in a unified fashion, to allow easier // tracking of active/inactive requests. diff --git a/src/req_wrap.h b/src/req_wrap.h index 8f8d0cf288..890eb6cb61 100644 --- a/src/req_wrap.h +++ b/src/req_wrap.h @@ -10,8 +10,24 @@ namespace node { +class ReqWrapBase { + public: + explicit inline ReqWrapBase(Environment* env); + + virtual ~ReqWrapBase() {} + + virtual void Cancel() = 0; + virtual AsyncWrap* GetAsyncWrap() = 0; + + private: + friend int GenDebugSymbols(); + friend class Environment; + + ListNode<ReqWrapBase> req_wrap_queue_; +}; + template <typename T> -class ReqWrap : public AsyncWrap { +class ReqWrap : public AsyncWrap, public ReqWrapBase { public: inline ReqWrap(Environment* env, v8::Local<v8::Object> object, @@ -23,7 +39,8 @@ class ReqWrap : public AsyncWrap { // Call this after a request has finished, if re-using this object is planned. inline void Reset(); T* req() { return &req_; } - inline void Cancel(); + inline void Cancel() final; + inline AsyncWrap* GetAsyncWrap() override; static ReqWrap* from_req(T* req); @@ -31,13 +48,10 @@ class ReqWrap : public AsyncWrap { inline int Dispatch(LibuvFunction fn, Args... args); private: - friend class Environment; friend int GenDebugSymbols(); template <typename ReqT, typename U> friend struct MakeLibuvRequestCallback; - ListNode<ReqWrap> req_wrap_queue_; - typedef void (*callback_t)(); callback_t original_callback_ = nullptr; |