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

src: allow generic C++ callables in SetImmediate() · nodejs/node@d601a0a · GitHub

/ node Public

Commit d601a0a

Browse files
addaleax
authored and
targos
committed
src: allow generic C++ callables in SetImmediate()
Modify the native `SetImmediate()` functions to take generic C++ callables as arguments. This makes passing arguments to the callback easier, and in particular, it allows passing `std::unique_ptr`s directly, which in turn makes sure that the data they point to is deleted if the `Environment` is torn down before the callback can run. PR-URL: #28704 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d9084d2 commit d601a0a

12 files changed

Lines changed: 206 additions & 163 deletions

‎src/async_wrap.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct AsyncWrapObject : public AsyncWrap {
8787
SET_SELF_SIZE(AsyncWrapObject)
8888
};
8989

90-
void AsyncWrap::DestroyAsyncIdsCallback(Environment* env, void* data) {
90+
void AsyncWrap::DestroyAsyncIdsCallback(Environment* env) {
9191
Local<Function> fn = env->async_hooks_destroy_function();
9292

9393
TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal);
@@ -642,7 +642,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
642642
}
643643

644644
if (env->destroy_async_id_list()->empty()) {
645-
env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr);
645+
env->SetUnrefImmediate(&DestroyAsyncIdsCallback);
646646
}
647647

648648
env->destroy_async_id_list()->push_back(async_id);

‎src/async_wrap.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ class AsyncWrap : public BaseObject {
154154
static void EmitTraceEventAfter(ProviderType type, double async_id);
155155
void EmitTraceEventDestroy();
156156

157-
static void DestroyAsyncIdsCallback(Environment* env, void* data);
157+
static void DestroyAsyncIdsCallback(Environment* env);
158158

159159
inline ProviderType provider_type() const;
160160
inline ProviderType set_provider_type(ProviderType provider);

‎src/cares_wrap.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -690,9 +690,9 @@ class QueryWrap : public AsyncWrap {
690690
}
691691

692692
void QueueResponseCallback(int status) {
693-
env()->SetImmediate([](Environment*, void* data) {
694-
static_cast<QueryWrap*>(data)->AfterResponse();
695-
}, this, object());
693+
env()->SetImmediate([this](Environment*) {
694+
AfterResponse();
695+
}, object());
696696

697697
channel_->set_query_last_ok(status != ARES_ECONNREFUSED);
698698
channel_->ModifyActivityQueryCount(-1);

‎src/env-inl.h‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -753,33 +753,66 @@ inline void IsolateData::set_options(
753753
options_ = std::move(options);
754754
}
755755

756-
void Environment::CreateImmediate(native_immediate_callback cb,
757-
void* data,
758-
v8::Local<v8::Object> obj,
756+
template <typename Fn>
757+
void Environment::CreateImmediate(Fn&& cb,
758+
v8::Local<v8::Object> keep_alive,
759759
bool ref) {
760-
native_immediate_callbacks_.push_back({
761-
cb,
762-
data,
763-
v8::Global<v8::Object>(isolate_, obj),
764-
ref
765-
});
760+
auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
761+
std::move(cb),
762+
v8::Global<v8::Object>(isolate(), keep_alive),
763+
ref);
764+
NativeImmediateCallback* prev_tail = native_immediate_callbacks_tail_;
765+
766+
native_immediate_callbacks_tail_ = callback.get();
767+
if (prev_tail != nullptr)
768+
prev_tail->set_next(std::move(callback));
769+
else
770+
native_immediate_callbacks_head_ = std::move(callback);
771+
766772
immediate_info()->count_inc(1);
767773
}
768774

769-
void Environment::SetImmediate(native_immediate_callback cb,
770-
void* data,
771-
v8::Local<v8::Object> obj) {
772-
CreateImmediate(cb, data, obj, true);
775+
template <typename Fn>
776+
void Environment::SetImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) {
777+
CreateImmediate(std::move(cb), keep_alive, true);
773778

774779
if (immediate_info()->ref_count() == 0)
775780
ToggleImmediateRef(true);
776781
immediate_info()->ref_count_inc(1);
777782
}
778783

779-
void Environment::SetUnrefImmediate(native_immediate_callback cb,
780-
void* data,
781-
v8::Local<v8::Object> obj) {
782-
CreateImmediate(cb, data, obj, false);
784+
template <typename Fn>
785+
void Environment::SetUnrefImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) {
786+
CreateImmediate(std::move(cb), keep_alive, false);
787+
}
788+
789+
Environment::NativeImmediateCallback::NativeImmediateCallback(bool refed)
790+
: refed_(refed) {}
791+
792+
bool Environment::NativeImmediateCallback::is_refed() const {
793+
return refed_;
794+
}
795+
796+
std::unique_ptr<Environment::NativeImmediateCallback>
797+
Environment::NativeImmediateCallback::get_next() {
798+
return std::move(next_);
799+
}
800+
801+
void Environment::NativeImmediateCallback::set_next(
802+
std::unique_ptr<NativeImmediateCallback> next) {
803+
next_ = std::move(next);
804+
}
805+
806+
template <typename Fn>
807+
Environment::NativeImmediateCallbackImpl<Fn>::NativeImmediateCallbackImpl(
808+
Fn&& callback, v8::Global<v8::Object>&& keep_alive, bool refed)
809+
: NativeImmediateCallback(refed),
810+
callback_(std::move(callback)),
811+
keep_alive_(std::move(keep_alive)) {}
812+
813+
template <typename Fn>
814+
void Environment::NativeImmediateCallbackImpl<Fn>::Call(Environment* env) {
815+
callback_(env);
783816
}
784817

785818
inline bool Environment::can_call_into_js() const {

‎src/env.cc‎

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ Environment::Environment(IsolateData* isolate_data,
339339
[](void* arg) {
340340
Environment* env = static_cast<Environment*>(arg);
341341
if (!env->destroy_async_id_list()->empty())
342-
AsyncWrap::DestroyAsyncIdsCallback(env, nullptr);
342+
AsyncWrap::DestroyAsyncIdsCallback(env);
343343
},
344344
this);
345345

@@ -642,42 +642,38 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) {
642642
void Environment::RunAndClearNativeImmediates() {
643643
TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
644644
"RunAndClearNativeImmediates", this);
645-
size_t count = native_immediate_callbacks_.size();
646-
if (count > 0) {
647-
size_t ref_count = 0;
648-
std::vector<NativeImmediateCallback> list;
649-
native_immediate_callbacks_.swap(list);
650-
auto drain_list = [&]() {
651-
TryCatchScope try_catch(this);
652-
for (auto it = list.begin(); it != list.end(); ++it) {
653-
DebugSealHandleScope seal_handle_scope(isolate());
654-
it->cb_(this, it->data_);
655-
if (it->refed_)
656-
ref_count++;
657-
if (UNLIKELY(try_catch.HasCaught())) {
658-
if (!try_catch.HasTerminated())
659-
errors::TriggerUncaughtException(isolate(), try_catch);
660-
661-
// We are done with the current callback. Increase the counter so that
662-
// the steps below make everything *after* the current item part of
663-
// the new list.
664-
it++;
665-
666-
// Bail out, remove the already executed callbacks from list
667-
// and set up a new TryCatch for the other pending callbacks.
668-
std::move_backward(it, list.end(), list.begin() + (list.end() - it));
669-
list.resize(list.end() - it);
670-
return true;
671-
}
645+
size_t ref_count = 0;
646+
size_t count = 0;
647+
std::unique_ptr<NativeImmediateCallback> head;
648+
head.swap(native_immediate_callbacks_head_);
649+
native_immediate_callbacks_tail_ = nullptr;
650+
651+
auto drain_list = [&]() {
652+
TryCatchScope try_catch(this);
653+
for (; head; head = head->get_next()) {
654+
DebugSealHandleScope seal_handle_scope(isolate());
655+
count++;
656+
if (head->is_refed())
657+
ref_count++;
658+
659+
head->Call(this);
660+
if (UNLIKELY(try_catch.HasCaught())) {
661+
if (!try_catch.HasTerminated())
662+
errors::TriggerUncaughtException(isolate(), try_catch);
663+
664+
// We are done with the current callback. Move one iteration along,
665+
// as if we had completed successfully.
666+
head = head->get_next();
667+
return true;
672668
}
673-
return false;
674-
};
675-
while (drain_list()) {}
669+
}
670+
return false;
671+
};
672+
while (head && drain_list()) {}
676673

677-
DCHECK_GE(immediate_info()->count(), count);
678-
immediate_info()->count_dec(count);
679-
immediate_info()->ref_count_dec(ref_count);
680-
}
674+
DCHECK_GE(immediate_info()->count(), count);
675+
immediate_info()->count_dec(count);
676+
immediate_info()->ref_count_dec(ref_count);
681677
}
682678

683679

‎src/env.h‎

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,15 +1153,15 @@ class Environment : public MemoryRetainer {
11531153
return current_value;
11541154
}
11551155

1156-
typedef void (*native_immediate_callback)(Environment* env, void* data);
1157-
// cb will be called as cb(env, data) on the next event loop iteration.
1158-
// obj will be kept alive between now and after the callback has run.
1159-
inline void SetImmediate(native_immediate_callback cb,
1160-
void* data,
1161-
v8::Local<v8::Object> obj = v8::Local<v8::Object>());
1162-
inline void SetUnrefImmediate(native_immediate_callback cb,
1163-
void* data,
1164-
v8::Local<v8::Object> obj =
1156+
// cb will be called as cb(env) on the next event loop iteration.
1157+
// keep_alive will be kept alive between now and after the callback has run.
1158+
template <typename Fn>
1159+
inline void SetImmediate(Fn&& cb,
1160+
v8::Local<v8::Object> keep_alive =
1161+
v8::Local<v8::Object>());
1162+
template <typename Fn>
1163+
inline void SetUnrefImmediate(Fn&& cb,
1164+
v8::Local<v8::Object> keep_alive =
11651165
v8::Local<v8::Object>());
11661166
// This needs to be available for the JS-land setImmediate().
11671167
void ToggleImmediateRef(bool ref);
@@ -1226,9 +1226,9 @@ class Environment : public MemoryRetainer {
12261226
#endif // HAVE_INSPECTOR
12271227

12281228
private:
1229-
inline void CreateImmediate(native_immediate_callback cb,
1230-
void* data,
1231-
v8::Local<v8::Object> obj,
1229+
template <typename Fn>
1230+
inline void CreateImmediate(Fn&& cb,
1231+
v8::Local<v8::Object> keep_alive,
12321232
bool ref);
12331233

12341234
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
@@ -1352,13 +1352,38 @@ class Environment : public MemoryRetainer {
13521352

13531353
std::list<ExitCallback> at_exit_functions_;
13541354

1355-
struct NativeImmediateCallback {
1356-
native_immediate_callback cb_;
1357-
void* data_;
1358-
v8::Global<v8::Object> keep_alive_;
1355+
class NativeImmediateCallback {
1356+
public:
1357+
explicit inline NativeImmediateCallback(bool refed);
1358+
1359+
virtual ~NativeImmediateCallback() = default;
1360+
virtual void Call(Environment* env) = 0;
1361+
1362+
inline bool is_refed() const;
1363+
inline std::unique_ptr<NativeImmediateCallback> get_next();
1364+
inline void set_next(std::unique_ptr<NativeImmediateCallback> next);
1365+
1366+
private:
13591367
bool refed_;
1368+
std::unique_ptr<NativeImmediateCallback> next_;
1369+
};
1370+
1371+
template <typename Fn>
1372+
class NativeImmediateCallbackImpl final : public NativeImmediateCallback {
1373+
public:
1374+
NativeImmediateCallbackImpl(Fn&& callback,
1375+
v8::Global<v8::Object>&& keep_alive,
1376+
bool refed);
1377+
void Call(Environment* env) override;
1378+
1379+
private:
1380+
Fn callback_;
1381+
v8::Global<v8::Object> keep_alive_;
13601382
};
1361-
std::vector<NativeImmediateCallback> native_immediate_callbacks_;
1383+
1384+
std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_;
1385+
NativeImmediateCallback* native_immediate_callbacks_tail_ = nullptr;
1386+
13621387
void RunAndClearNativeImmediates();
13631388
static void CheckImmediate(uv_check_t* handle);
13641389

‎src/node_api.cc‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,33 @@ class BufferFinalizer : private Finalizer {
3636
public:
3737
// node::Buffer::FreeCallback
3838
static void FinalizeBufferCallback(char* data, void* hint) {
39-
BufferFinalizer* finalizer = static_cast<BufferFinalizer*>(hint);
39+
std::unique_ptr<BufferFinalizer, Deleter> finalizer{
40+
static_cast<BufferFinalizer*>(hint)};
4041
finalizer->_finalize_data = data;
41-
static_cast<node_napi_env>(finalizer->_env)->node_env()
42-
->SetImmediate([](node::Environment* env, void* hint) {
43-
BufferFinalizer* finalizer = static_cast<BufferFinalizer*>(hint);
44-
45-
if (finalizer->_finalize_callback != nullptr) {
46-
v8::HandleScope handle_scope(finalizer->_env->isolate);
47-
v8::Context::Scope context_scope(finalizer->_env->context());
48-
49-
finalizer->_env->CallIntoModuleThrow([&](napi_env env) {
50-
finalizer->_finalize_callback(
51-
env,
52-
finalizer->_finalize_data,
53-
finalizer->_finalize_hint);
54-
});
55-
}
5642

57-
Delete(finalizer);
58-
}, hint);
43+
node::Environment* node_env =
44+
static_cast<node_napi_env>(finalizer->_env)->node_env();
45+
node_env->SetImmediate(
46+
[finalizer = std::move(finalizer)](node::Environment* env) {
47+
if (finalizer->_finalize_callback == nullptr) return;
48+
49+
v8::HandleScope handle_scope(finalizer->_env->isolate);
50+
v8::Context::Scope context_scope(finalizer->_env->context());
51+
52+
finalizer->_env->CallIntoModuleThrow([&](napi_env env) {
53+
finalizer->_finalize_callback(
54+
env,
55+
finalizer->_finalize_data,
56+
finalizer->_finalize_hint);
57+
});
58+
});
5959
}
60+
61+
struct Deleter {
62+
void operator()(BufferFinalizer* finalizer) {
63+
Finalizer::Delete(finalizer);
64+
}
65+
};
6066
};
6167

6268
static inline napi_env NewEnv(v8::Local<v8::Context> context) {

‎src/node_file.cc‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,35 +170,33 @@ inline void FileHandle::Close() {
170170

171171
struct err_detail { int ret; int fd; };
172172

173-
err_detail* detail = new err_detail { ret, fd_ };
173+
err_detail detail { ret, fd_ };
174174

175175
if (ret < 0) {
176176
// Do not unref this
177-
env()->SetImmediate([](Environment* env, void* data) {
177+
env()->SetImmediate([detail](Environment* env) {
178178
char msg[70];
179-
std::unique_ptr<err_detail> detail(static_cast<err_detail*>(data));
180179
snprintf(msg, arraysize(msg),
181180
"Closing file descriptor %d on garbage collection failed",
182-
detail->fd);
181+
detail.fd);
183182
// This exception will end up being fatal for the process because
184183
// it is being thrown from within the SetImmediate handler and
185184
// there is no JS stack to bubble it to. In other words, tearing
186185
// down the process is the only reasonable thing we can do here.
187186
HandleScope handle_scope(env->isolate());
188-
env->ThrowUVException(detail->ret, "close", msg);
189-
}, detail);
187+
env->ThrowUVException(detail.ret, "close", msg);
188+
});
190189
return;
191190
}
192191

193192
// If the close was successful, we still want to emit a process warning
194193
// to notify that the file descriptor was gc'd. We want to be noisy about
195194
// this because not explicitly closing the FileHandle is a bug.
196-
env()->SetUnrefImmediate([](Environment* env, void* data) {
197-
std::unique_ptr<err_detail> detail(static_cast<err_detail*>(data));
195+
env()->SetUnrefImmediate([detail](Environment* env) {
198196
ProcessEmitWarning(env,
199197
"Closing file descriptor %d on garbage collection",
200-
detail->fd);
201-
}, detail);
198+
detail.fd);
199+
});
202200
}
203201

204202
void FileHandle::CloseReq::Resolve() {

0 commit comments

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