Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: move req_wrap_queue to base class of ReqWrap #26148

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,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_)
Expand Down
3 changes: 1 addition & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,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_; }
Expand Down
8 changes: 5 additions & 3 deletions src/node_postmortem_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,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());
Expand Down
17 changes: 11 additions & 6 deletions src/req_wrap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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.
Expand Down
24 changes: 19 additions & 5 deletions src/req_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,21 +39,19 @@ 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);

template <typename LibuvFunction, typename... Args>
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;

Expand Down