Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8995408

Browse filesBrowse files
addaleaxaddaleax
committed
src: keep track of open requests
Workers cannot shut down while requests are open, so keep a counter that is increased whenever libuv requests are made and decreased whenever their callback is called. This also applies to other embedders, who may want to shut down an `Environment` instance early. Many thanks for Stephen Belanger for reviewing the original version of this commit in the Ayo.js project. Fixes: #20517 Refs: ayojs/ayo#85 PR-URL: #19377 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 75aad90 commit 8995408
Copy full SHA for 8995408

File tree

Expand file treeCollapse file tree

9 files changed

+83
-24
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+83
-24
lines changed
Open diff view settings
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,15 @@ inline void Environment::CloseHandle(T* handle, OnCloseCallback callback) {
371371
});
372372
}
373373

374+
void Environment::IncreaseWaitingRequestCounter() {
375+
request_waiting_++;
376+
}
377+
378+
void Environment::DecreaseWaitingRequestCounter() {
379+
request_waiting_--;
380+
CHECK_GE(request_waiting_, 0);
381+
}
382+
374383
inline uv_loop_t* Environment::event_loop() const {
375384
return isolate_data()->event_loop();
376385
}
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ Environment::Environment(IsolateData* isolate_data,
107107
#if HAVE_INSPECTOR
108108
inspector_agent_(new inspector::Agent(this)),
109109
#endif
110-
handle_cleanup_waiting_(0),
111110
http_parser_buffer_(nullptr),
112111
fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2),
113112
context_(context->GetIsolate(), context) {
@@ -241,8 +240,11 @@ void Environment::CleanupHandles() {
241240
hc.cb_(this, hc.handle_, hc.arg_);
242241
handle_cleanup_queue_.clear();
243242

244-
while (handle_cleanup_waiting_ != 0 || !handle_wrap_queue_.IsEmpty())
243+
while (handle_cleanup_waiting_ != 0 ||
244+
request_waiting_ != 0 ||
245+
!handle_wrap_queue_.IsEmpty()) {
245246
uv_run(event_loop(), UV_RUN_ONCE);
247+
}
246248
}
247249

248250
void Environment::StartProfilerIdleNotifier() {
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,9 @@ class Environment {
601601
inline uv_check_t* immediate_check_handle();
602602
inline uv_idle_t* immediate_idle_handle();
603603

604+
inline void IncreaseWaitingRequestCounter();
605+
inline void DecreaseWaitingRequestCounter();
606+
604607
inline AsyncHooks* async_hooks();
605608
inline ImmediateInfo* immediate_info();
606609
inline TickInfo* tick_info();
@@ -833,7 +836,8 @@ class Environment {
833836
HandleWrapQueue handle_wrap_queue_;
834837
ReqWrapQueue req_wrap_queue_;
835838
std::list<HandleCleanup> handle_cleanup_queue_;
836-
int handle_cleanup_waiting_;
839+
int handle_cleanup_waiting_ = 0;
840+
int request_waiting_ = 0;
837841

838842
double* heap_statistics_buffer_ = nullptr;
839843
double* heap_space_statistics_buffer_ = nullptr;
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3388,6 +3388,9 @@ class Work : public node::AsyncResource {
33883388
// Establish a handle scope here so that every callback doesn't have to.
33893389
// Also it is needed for the exception-handling below.
33903390
v8::HandleScope scope(env->isolate);
3391+
node::Environment* env_ = node::Environment::GetCurrent(env->isolate);
3392+
env_->DecreaseWaitingRequestCounter();
3393+
33913394
CallbackScope callback_scope(work);
33923395

33933396
NAPI_CALL_INTO_MODULE(env,
@@ -3488,6 +3491,8 @@ napi_status napi_queue_async_work(napi_env env, napi_async_work work) {
34883491

34893492
uvimpl::Work* w = reinterpret_cast<uvimpl::Work*>(work);
34903493

3494+
node::Environment* env_ = node::Environment::GetCurrent(env->isolate);
3495+
env_->IncreaseWaitingRequestCounter();
34913496
CALL_UV(env, uv_queue_work(event_loop,
34923497
w->Request(),
34933498
uvimpl::Work::ExecuteCallback,
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+11-2Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4645,9 +4645,12 @@ void PBKDF2Request::After() {
46454645

46464646

46474647
void PBKDF2Request::After(uv_work_t* work_req, int status) {
4648-
CHECK_EQ(status, 0);
46494648
std::unique_ptr<PBKDF2Request> req(
46504649
ContainerOf(&PBKDF2Request::work_req_, work_req));
4650+
req->env()->DecreaseWaitingRequestCounter();
4651+
if (status == UV_ECANCELED)
4652+
return;
4653+
CHECK_EQ(status, 0);
46514654
req->After();
46524655
}
46534656

@@ -4698,6 +4701,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
46984701
if (args[5]->IsFunction()) {
46994702
obj->Set(env->context(), env->ondone_string(), args[5]).FromJust();
47004703

4704+
env->IncreaseWaitingRequestCounter();
47014705
uv_queue_work(env->event_loop(),
47024706
req.release()->work_req(),
47034707
PBKDF2Request::Work,
@@ -4837,10 +4841,13 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> (*argv)[2]) {
48374841

48384842

48394843
void RandomBytesAfter(uv_work_t* work_req, int status) {
4840-
CHECK_EQ(status, 0);
48414844
std::unique_ptr<RandomBytesRequest> req(
48424845
ContainerOf(&RandomBytesRequest::work_req_, work_req));
48434846
Environment* env = req->env();
4847+
env->DecreaseWaitingRequestCounter();
4848+
if (status == UV_ECANCELED)
4849+
return;
4850+
CHECK_EQ(status, 0);
48444851
HandleScope handle_scope(env->isolate());
48454852
Context::Scope context_scope(env->context());
48464853
Local<Value> argv[2];
@@ -4880,6 +4887,7 @@ void RandomBytes(const FunctionCallbackInfo<Value>& args) {
48804887
if (args[1]->IsFunction()) {
48814888
obj->Set(env->context(), env->ondone_string(), args[1]).FromJust();
48824889

4890+
env->IncreaseWaitingRequestCounter();
48834891
uv_queue_work(env->event_loop(),
48844892
req.release()->work_req(),
48854893
RandomBytesWork,
@@ -4919,6 +4927,7 @@ void RandomBytesBuffer(const FunctionCallbackInfo<Value>& args) {
49194927
if (args[3]->IsFunction()) {
49204928
obj->Set(env->context(), env->ondone_string(), args[3]).FromJust();
49214929

4930+
env->IncreaseWaitingRequestCounter();
49224931
uv_queue_work(env->event_loop(),
49234932
req.release()->work_req(),
49244933
RandomBytesWork,
Collapse file

‎src/node_zlib.cc‎

Copy file name to clipboardExpand all lines: src/node_zlib.cc
+10-3Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ class ZCtx : public AsyncWrap {
214214
}
215215

216216
// async version
217+
env->IncreaseWaitingRequestCounter();
217218
uv_queue_work(env->event_loop(), work_req, ZCtx::Process, ZCtx::After);
218219
}
219220

@@ -361,10 +362,17 @@ class ZCtx : public AsyncWrap {
361362

362363
// v8 land!
363364
static void After(uv_work_t* work_req, int status) {
364-
CHECK_EQ(status, 0);
365-
366365
ZCtx* ctx = ContainerOf(&ZCtx::work_req_, work_req);
367366
Environment* env = ctx->env();
367+
ctx->write_in_progress_ = false;
368+
369+
env->DecreaseWaitingRequestCounter();
370+
if (status == UV_ECANCELED) {
371+
ctx->Close();
372+
return;
373+
}
374+
375+
CHECK_EQ(status, 0);
368376

369377
HandleScope handle_scope(env->isolate());
370378
Context::Scope context_scope(env->context());
@@ -374,7 +382,6 @@ class ZCtx : public AsyncWrap {
374382

375383
ctx->write_result_[0] = ctx->strm_.avail_out;
376384
ctx->write_result_[1] = ctx->strm_.avail_in;
377-
ctx->write_in_progress_ = false;
378385

379386
// call the write() cb
380387
Local<Function> cb = PersistentToLocal(env->isolate(),
Collapse file

‎src/req_wrap-inl.h‎

Copy file name to clipboardExpand all lines: src/req_wrap-inl.h
+28-15Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ ReqWrap<T>::ReqWrap(Environment* env,
2020
// FIXME(bnoordhuis) The fact that a reinterpret_cast is needed is
2121
// arguably a good indicator that there should be more than one queue.
2222
env->req_wrap_queue()->PushBack(reinterpret_cast<ReqWrap<uv_req_t>*>(this));
23+
24+
Reset();
2325
}
2426

2527
template <typename T>
@@ -33,14 +35,21 @@ void ReqWrap<T>::Dispatched() {
3335
req_.data = this;
3436
}
3537

38+
template <typename T>
39+
void ReqWrap<T>::Reset() {
40+
original_callback_ = nullptr;
41+
req_.data = nullptr;
42+
}
43+
3644
template <typename T>
3745
ReqWrap<T>* ReqWrap<T>::from_req(T* req) {
3846
return ContainerOf(&ReqWrap<T>::req_, req);
3947
}
4048

4149
template <typename T>
4250
void ReqWrap<T>::Cancel() {
43-
uv_cancel(reinterpret_cast<uv_req_t*>(&req_));
51+
if (req_.data == this) // Only cancel if already dispatched.
52+
uv_cancel(reinterpret_cast<uv_req_t*>(&req_));
4453
}
4554

4655
// Below is dark template magic designed to invoke libuv functions that
@@ -95,7 +104,7 @@ struct CallLibuvFunction<ReqT, void(*)(ReqT*, Args...)> {
95104
template <typename ReqT, typename T>
96105
struct MakeLibuvRequestCallback {
97106
static T For(ReqWrap<ReqT>* req_wrap, T v) {
98-
static_assert(!std::is_function<T>::value,
107+
static_assert(!is_callable<T>::value,
99108
"MakeLibuvRequestCallback missed a callback");
100109
return v;
101110
}
@@ -109,6 +118,7 @@ struct MakeLibuvRequestCallback<ReqT, void(*)(ReqT*, Args...)> {
109118

110119
static void Wrapper(ReqT* req, Args... args) {
111120
ReqWrap<ReqT>* req_wrap = ContainerOf(&ReqWrap<ReqT>::req_, req);
121+
req_wrap->env()->DecreaseWaitingRequestCounter();
112122
F original_callback = reinterpret_cast<F>(req_wrap->original_callback_);
113123
original_callback(req, args...);
114124
}
@@ -128,23 +138,26 @@ int ReqWrap<T>::Dispatch(LibuvFunction fn, Args... args) {
128138

129139
// This expands as:
130140
//
131-
// return fn(env()->event_loop(), req(), arg1, arg2, Wrapper, arg3, ...)
132-
// ^ ^ ^
133-
// | | |
134-
// \-- Omitted if `fn` has no | |
135-
// first `uv_loop_t*` argument | |
136-
// | |
137-
// A function callback whose first argument | |
138-
// matches the libuv request type is replaced ---/ |
139-
// by the `Wrapper` method defined above |
140-
// |
141-
// Other (non-function) arguments are passed -----/
142-
// through verbatim
143-
return CallLibuvFunction<T, LibuvFunction>::Call(
141+
// int err = fn(env()->event_loop(), req(), arg1, arg2, Wrapper, arg3, ...)
142+
// ^ ^ ^
143+
// | | |
144+
// \-- Omitted if `fn` has no | |
145+
// first `uv_loop_t*` argument | |
146+
// | |
147+
// A function callback whose first argument | |
148+
// matches the libuv request type is replaced ---/ |
149+
// by the `Wrapper` method defined above |
150+
// |
151+
// Other (non-function) arguments are passed -----/
152+
// through verbatim
153+
int err = CallLibuvFunction<T, LibuvFunction>::Call(
144154
fn,
145155
env()->event_loop(),
146156
req(),
147157
MakeLibuvRequestCallback<T, Args>::For(this, args)...);
158+
if (err >= 0)
159+
env()->IncreaseWaitingRequestCounter();
160+
return err;
148161
}
149162

150163
} // namespace node
Collapse file

‎src/req_wrap.h‎

Copy file name to clipboardExpand all lines: src/req_wrap.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ class ReqWrap : public AsyncWrap {
2020
// Call this after the req has been dispatched, if that did not already
2121
// happen by using Dispatch().
2222
inline void Dispatched();
23+
// Call this after a request has finished, if re-using this object is planned.
24+
inline void Reset();
2325
T* req() { return &req_; }
2426
inline void Cancel();
2527

Collapse file

‎src/util.h‎

Copy file name to clipboardExpand all lines: src/util.h
+9-1Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,16 @@ struct MallocedBuffer {
447447
MallocedBuffer& operator=(const MallocedBuffer&) = delete;
448448
};
449449

450-
} // namespace node
450+
// Test whether some value can be called with ().
451+
template<typename T, typename = void>
452+
struct is_callable : std::is_function<T> { };
453+
454+
template<typename T>
455+
struct is_callable<T, typename std::enable_if<
456+
std::is_same<decltype(void(&T::operator())), void>::value
457+
>::type> : std::true_type { };
451458

459+
} // namespace node
452460

453461
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
454462

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.