FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: move req_wrap_queue to base class of ReqWrap · nodejs/node@c6d5af5 · GitHub

/ node Public

Commit c6d5af5

Browse files
addaleax
authored and
rvagg
committed
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: #26131 PR-URL: #26148 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 007b2fa commit c6d5af5

6 files changed

Lines changed: 39 additions & 18 deletions

File tree

‎src/env.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ void Environment::RegisterHandleCleanups() {
420420
}
421421

422422
void Environment::CleanupHandles() {
423-
for (ReqWrap<uv_req_t>* request : req_wrap_queue_)
423+
for (ReqWrapBase* request : req_wrap_queue_)
424424
request->Cancel();
425425

426426
for (HandleWrap* handle : handle_wrap_queue_)

‎src/env.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,8 +877,7 @@ class Environment {
877877
#endif
878878

879879
typedef ListHead<HandleWrap, &HandleWrap::handle_wrap_queue_> HandleWrapQueue;
880-
typedef ListHead<ReqWrap<uv_req_t>, &ReqWrap<uv_req_t>::req_wrap_queue_>
881-
ReqWrapQueue;
880+
typedef ListHead<ReqWrapBase, &ReqWrapBase::req_wrap_queue_> ReqWrapQueue;
882881

883882
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
884883
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }

‎src/node_postmortem_metadata.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,14 @@
2828
V(Environment_HandleWrapQueue, head_, ListNode_HandleWrap, \
2929
Environment::HandleWrapQueue::head_) \
3030
V(ListNode_HandleWrap, next_, uintptr_t, ListNode<HandleWrap>::next_) \
31-
V(ReqWrap, req_wrap_queue_, ListNode_ReqWrapQueue, \
32-
ReqWrap<uv_req_t>::req_wrap_queue_) \
3331
V(Environment_ReqWrapQueue, head_, ListNode_ReqWrapQueue, \
3432
Environment::ReqWrapQueue::head_) \
35-
V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrap<uv_req_t>>::next_)
33+
V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrapBase>::next_)
3634

3735
extern "C" {
3836
int nodedbg_const_ContextEmbedderIndex__kEnvironment__int;
3937
uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;
38+
uintptr_t nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue;
4039

4140
#define V(Class, Member, Type, Accessor) \
4241
NODE_EXTERN uintptr_t NODEDBG_OFFSET(Class, Member, Type);
@@ -51,6 +50,9 @@ int GenDebugSymbols() {
5150
ContextEmbedderIndex::kEnvironment;
5251

5352
nodedbg_offset_ExternalString__data__uintptr_t = NODE_OFF_EXTSTR_DATA;
53+
nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue =
54+
OffsetOf<ListNode<ReqWrapBase>, ReqWrap<uv_req_t>>(
55+
&ReqWrap<uv_req_t>::req_wrap_queue_);
5456

5557
#define V(Class, Member, Type, Accessor) \
5658
NODEDBG_OFFSET(Class, Member, Type) = OffsetOf(&Accessor);

‎src/node_process_methods.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
252252
Environment* env = Environment::GetCurrent(args);
253253

254254
std::vector<Local<Value>> request_v;
255-
for (auto w : *env->req_wrap_queue()) {
255+
for (ReqWrapBase* req_wrap : *env->req_wrap_queue()) {
256+
AsyncWrap* w = req_wrap->GetAsyncWrap();
256257
if (w->persistent().IsEmpty())
257258
continue;
258259
request_v.push_back(w->GetOwner());

‎src/req_wrap-inl.h‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,16 @@
1111

1212
namespace node {
1313

14+
ReqWrapBase::ReqWrapBase(Environment* env) {
15+
env->req_wrap_queue()->PushBack(this);
16+
}
17+
1418
template <typename T>
1519
ReqWrap<T>::ReqWrap(Environment* env,
1620
v8::Local<v8::Object> object,
1721
AsyncWrap::ProviderType provider)
18-
: AsyncWrap(env, object, provider) {
19-
20-
// FIXME(bnoordhuis) The fact that a reinterpret_cast is needed is
21-
// arguably a good indicator that there should be more than one queue.
22-
env->req_wrap_queue()->PushBack(reinterpret_cast<ReqWrap<uv_req_t>*>(this));
23-
22+
: AsyncWrap(env, object, provider),
23+
ReqWrapBase(env) {
2424
Reset();
2525
}
2626

@@ -51,6 +51,11 @@ void ReqWrap<T>::Cancel() {
5151
uv_cancel(reinterpret_cast<uv_req_t*>(&req_));
5252
}
5353

54+
template <typename T>
55+
AsyncWrap* ReqWrap<T>::GetAsyncWrap() {
56+
return this;
57+
}
58+
5459
// Below is dark template magic designed to invoke libuv functions that
5560
// initialize uv_req_t instances in a unified fashion, to allow easier
5661
// tracking of active/inactive requests.

‎src/req_wrap.h‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,24 @@
1010

1111
namespace node {
1212

13+
class ReqWrapBase {
14+
public:
15+
explicit inline ReqWrapBase(Environment* env);
16+
17+
virtual ~ReqWrapBase() {}
18+
19+
virtual void Cancel() = 0;
20+
virtual AsyncWrap* GetAsyncWrap() = 0;
21+
22+
private:
23+
friend int GenDebugSymbols();
24+
friend class Environment;
25+
26+
ListNode<ReqWrapBase> req_wrap_queue_;
27+
};
28+
1329
template <typename T>
14-
class ReqWrap : public AsyncWrap {
30+
class ReqWrap : public AsyncWrap, public ReqWrapBase {
1531
public:
1632
inline ReqWrap(Environment* env,
1733
v8::Local<v8::Object> object,
@@ -23,21 +39,19 @@ class ReqWrap : public AsyncWrap {
2339
// Call this after a request has finished, if re-using this object is planned.
2440
inline void Reset();
2541
T* req() { return &req_; }
26-
inline void Cancel();
42+
inline void Cancel() final;
43+
inline AsyncWrap* GetAsyncWrap() override;
2744

2845
static ReqWrap* from_req(T* req);
2946

3047
template <typename LibuvFunction, typename... Args>
3148
inline int Dispatch(LibuvFunction fn, Args... args);
3249

3350
private:
34-
friend class Environment;
3551
friend int GenDebugSymbols();
3652
template <typename ReqT, typename U>
3753
friend struct MakeLibuvRequestCallback;
3854

39-
ListNode<ReqWrap> req_wrap_queue_;
40-
4155
typedef void (*callback_t)();
4256
callback_t original_callback_ = nullptr;
4357

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL