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

src: remove keep alive option from SetImmediate() · nodejs/node@4222f24 · GitHub

/ node Public

Commit 4222f24

Browse files
addaleax
authored and
MylesBorins
committed
src: remove keep alive option from SetImmediate()
This is no longer necessary now that the copyable `BaseObjectPtr` is available (as opposed to the only-movable `v8::Global`). PR-URL: #30374 Refs: nodejs/quic#141 Refs: nodejs/quic#149 Refs: nodejs/quic#141 Refs: nodejs/quic#165 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
1 parent 940a297 commit 4222f24

6 files changed

Lines changed: 39 additions & 43 deletions

File tree

‎src/cares_wrap.cc‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -627,8 +627,6 @@ class QueryWrap : public AsyncWrap {
627627
} else {
628628
Parse(response_data_->host.get());
629629
}
630-
631-
delete this;
632630
}
633631

634632
void* MakeCallbackPointer() {
@@ -686,9 +684,13 @@ class QueryWrap : public AsyncWrap {
686684
}
687685

688686
void QueueResponseCallback(int status) {
689-
env()->SetImmediate([this](Environment*) {
687+
BaseObjectPtr<QueryWrap> strong_ref{this};
688+
env()->SetImmediate([this, strong_ref](Environment*) {
690689
AfterResponse();
691-
}, object());
690+
691+
// Delete once strong_ref goes out of scope.
692+
Detach();
693+
});
692694

693695
channel_->set_query_last_ok(status != ARES_ECONNREFUSED);
694696
channel_->ModifyActivityQueryCount(-1);

‎src/env-inl.h‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -746,13 +746,9 @@ inline void IsolateData::set_options(
746746
}
747747

748748
template <typename Fn>
749-
void Environment::CreateImmediate(Fn&& cb,
750-
v8::Local<v8::Object> keep_alive,
751-
bool ref) {
749+
void Environment::CreateImmediate(Fn&& cb, bool ref) {
752750
auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
753-
std::move(cb),
754-
v8::Global<v8::Object>(isolate(), keep_alive),
755-
ref);
751+
std::move(cb), ref);
756752
NativeImmediateCallback* prev_tail = native_immediate_callbacks_tail_;
757753

758754
native_immediate_callbacks_tail_ = callback.get();
@@ -765,17 +761,17 @@ void Environment::CreateImmediate(Fn&& cb,
765761
}
766762

767763
template <typename Fn>
768-
void Environment::SetImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) {
769-
CreateImmediate(std::move(cb), keep_alive, true);
764+
void Environment::SetImmediate(Fn&& cb) {
765+
CreateImmediate(std::move(cb), true);
770766

771767
if (immediate_info()->ref_count() == 0)
772768
ToggleImmediateRef(true);
773769
immediate_info()->ref_count_inc(1);
774770
}
775771

776772
template <typename Fn>
777-
void Environment::SetUnrefImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) {
778-
CreateImmediate(std::move(cb), keep_alive, false);
773+
void Environment::SetUnrefImmediate(Fn&& cb) {
774+
CreateImmediate(std::move(cb), false);
779775
}
780776

781777
Environment::NativeImmediateCallback::NativeImmediateCallback(bool refed)
@@ -797,10 +793,9 @@ void Environment::NativeImmediateCallback::set_next(
797793

798794
template <typename Fn>
799795
Environment::NativeImmediateCallbackImpl<Fn>::NativeImmediateCallbackImpl(
800-
Fn&& callback, v8::Global<v8::Object>&& keep_alive, bool refed)
796+
Fn&& callback, bool refed)
801797
: NativeImmediateCallback(refed),
802-
callback_(std::move(callback)),
803-
keep_alive_(std::move(keep_alive)) {}
798+
callback_(std::move(callback)) {}
804799

805800
template <typename Fn>
806801
void Environment::NativeImmediateCallbackImpl<Fn>::Call(Environment* env) {

‎src/env.h‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,13 +1183,9 @@ class Environment : public MemoryRetainer {
11831183
// cb will be called as cb(env) on the next event loop iteration.
11841184
// keep_alive will be kept alive between now and after the callback has run.
11851185
template <typename Fn>
1186-
inline void SetImmediate(Fn&& cb,
1187-
v8::Local<v8::Object> keep_alive =
1188-
v8::Local<v8::Object>());
1186+
inline void SetImmediate(Fn&& cb);
11891187
template <typename Fn>
1190-
inline void SetUnrefImmediate(Fn&& cb,
1191-
v8::Local<v8::Object> keep_alive =
1192-
v8::Local<v8::Object>());
1188+
inline void SetUnrefImmediate(Fn&& cb);
11931189
// This needs to be available for the JS-land setImmediate().
11941190
void ToggleImmediateRef(bool ref);
11951191

@@ -1260,9 +1256,7 @@ class Environment : public MemoryRetainer {
12601256

12611257
private:
12621258
template <typename Fn>
1263-
inline void CreateImmediate(Fn&& cb,
1264-
v8::Local<v8::Object> keep_alive,
1265-
bool ref);
1259+
inline void CreateImmediate(Fn&& cb, bool ref);
12661260

12671261
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
12681262
const char* errmsg);
@@ -1410,14 +1404,11 @@ class Environment : public MemoryRetainer {
14101404
template <typename Fn>
14111405
class NativeImmediateCallbackImpl final : public NativeImmediateCallback {
14121406
public:
1413-
NativeImmediateCallbackImpl(Fn&& callback,
1414-
v8::Global<v8::Object>&& keep_alive,
1415-
bool refed);
1407+
NativeImmediateCallbackImpl(Fn&& callback, bool refed);
14161408
void Call(Environment* env) override;
14171409

14181410
private:
14191411
Fn callback_;
1420-
v8::Global<v8::Object> keep_alive_;
14211412
};
14221413

14231414
std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_;

‎src/node_http2.cc‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,7 +1582,8 @@ void Http2Session::MaybeScheduleWrite() {
15821582
HandleScope handle_scope(env()->isolate());
15831583
Debug(this, "scheduling write");
15841584
flags_ |= SESSION_STATE_WRITE_SCHEDULED;
1585-
env()->SetImmediate([this](Environment* env) {
1585+
BaseObjectPtr<Http2Session> strong_ref{this};
1586+
env()->SetImmediate([this, strong_ref](Environment* env) {
15861587
if (session_ == nullptr || !(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
15871588
// This can happen e.g. when a stream was reset before this turn
15881589
// of the event loop, in which case SendPendingData() is called early,
@@ -1595,7 +1596,7 @@ void Http2Session::MaybeScheduleWrite() {
15951596
HandleScope handle_scope(env->isolate());
15961597
InternalCallbackScope callback_scope(this);
15971598
SendPendingData();
1598-
}, object());
1599+
});
15991600
}
16001601
}
16011602

@@ -2043,7 +2044,8 @@ void Http2Stream::Destroy() {
20432044

20442045
// Wait until the start of the next loop to delete because there
20452046
// may still be some pending operations queued for this stream.
2046-
env()->SetImmediate([this](Environment* env) {
2047+
BaseObjectPtr<Http2Stream> strong_ref{this};
2048+
env()->SetImmediate([this, strong_ref](Environment* env) {
20472049
// Free any remaining outgoing data chunks here. This should be done
20482050
// here because it's possible for destroy to have been called while
20492051
// we still have queued outbound writes.
@@ -2057,9 +2059,11 @@ void Http2Stream::Destroy() {
20572059
// We can destroy the stream now if there are no writes for it
20582060
// already on the socket. Otherwise, we'll wait for the garbage collector
20592061
// to take care of cleaning up.
2060-
if (session() == nullptr || !session()->HasWritesOnSocketForStream(this))
2061-
delete this;
2062-
}, object());
2062+
if (session() == nullptr || !session()->HasWritesOnSocketForStream(this)) {
2063+
// Delete once strong_ref goes out of scope.
2064+
Detach();
2065+
}
2066+
});
20632067

20642068
statistics_.end_time = uv_hrtime();
20652069
session_->statistics_.stream_average_duration =

‎src/stream_pipe.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ void StreamPipe::Unpipe() {
7171
// Delay the JS-facing part with SetImmediate, because this might be from
7272
// inside the garbage collector, so we can’t run JS here.
7373
HandleScope handle_scope(env()->isolate());
74+
BaseObjectPtr<StreamPipe> strong_ref{this};
7475
env()->SetImmediate([this](Environment* env) {
7576
HandleScope handle_scope(env->isolate());
7677
Context::Scope context_scope(env->context());
@@ -105,7 +106,7 @@ void StreamPipe::Unpipe() {
105106
.IsNothing()) {
106107
return;
107108
}
108-
}, object());
109+
});
109110
}
110111

111112
uv_buf_t StreamPipe::ReadableListener::OnStreamAlloc(size_t suggested_size) {

‎src/tls_wrap.cc‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,10 @@ void TLSWrap::EncOut() {
316316
// its not clear if it is always correct. Not calling Done() could block
317317
// data flow, so for now continue to call Done(), just do it in the next
318318
// tick.
319-
env()->SetImmediate([this](Environment* env) {
319+
BaseObjectPtr<TLSWrap> strong_ref{this};
320+
env()->SetImmediate([this, strong_ref](Environment* env) {
320321
InvokeQueued(0);
321-
}, object());
322+
});
322323
}
323324
}
324325
return;
@@ -349,9 +350,10 @@ void TLSWrap::EncOut() {
349350
HandleScope handle_scope(env()->isolate());
350351

351352
// Simulate asynchronous finishing, TLS cannot handle this at the moment.
352-
env()->SetImmediate([this](Environment* env) {
353+
BaseObjectPtr<TLSWrap> strong_ref{this};
354+
env()->SetImmediate([this, strong_ref](Environment* env) {
353355
OnStreamAfterWrite(nullptr, 0);
354-
}, object());
356+
});
355357
}
356358
}
357359

@@ -718,9 +720,10 @@ int TLSWrap::DoWrite(WriteWrap* w,
718720
StreamWriteResult res =
719721
underlying_stream()->Write(bufs, count, send_handle);
720722
if (!res.async) {
721-
env()->SetImmediate([this](Environment* env) {
723+
BaseObjectPtr<TLSWrap> strong_ref{this};
724+
env()->SetImmediate([this, strong_ref](Environment* env) {
722725
OnStreamAfterWrite(current_empty_write_, 0);
723-
}, object());
726+
});
724727
}
725728
return 0;
726729
}

0 commit comments

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