diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-11-12 14:01:08 +0000 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-11-19 13:47:33 +0100 |
commit | a489583eda4d7cebc06516834b31dc2a4cedb1b6 (patch) | |
tree | 8ead818f1ed28ce7c8965917209abc8d5b85bcb0 /src | |
parent | c0b5e6fd4b87d1e61151b77b3ec10e6650f5153b (diff) | |
download | android-node-v8-a489583eda4d7cebc06516834b31dc2a4cedb1b6.tar.gz android-node-v8-a489583eda4d7cebc06516834b31dc2a4cedb1b6.tar.bz2 android-node-v8-a489583eda4d7cebc06516834b31dc2a4cedb1b6.zip |
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: https://github.com/nodejs/node/pull/30374
Refs: https://github.com/nodejs/quic/pull/141
Refs: https://github.com/nodejs/quic/pull/149
Refs: https://github.com/nodejs/quic/pull/141
Refs: https://github.com/nodejs/quic/pull/165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/cares_wrap.cc | 10 | ||||
-rw-r--r-- | src/env-inl.h | 21 | ||||
-rw-r--r-- | src/env.h | 17 | ||||
-rw-r--r-- | src/node_http2.cc | 16 | ||||
-rw-r--r-- | src/stream_pipe.cc | 3 | ||||
-rw-r--r-- | src/tls_wrap.cc | 15 |
6 files changed, 39 insertions, 43 deletions
diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index ee521ce64a..1fb0f47dd8 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -627,8 +627,6 @@ class QueryWrap : public AsyncWrap { } else { Parse(response_data_->host.get()); } - - delete this; } void* MakeCallbackPointer() { @@ -686,9 +684,13 @@ class QueryWrap : public AsyncWrap { } void QueueResponseCallback(int status) { - env()->SetImmediate([this](Environment*) { + BaseObjectPtr<QueryWrap> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment*) { AfterResponse(); - }, object()); + + // Delete once strong_ref goes out of scope. + Detach(); + }); channel_->set_query_last_ok(status != ARES_ECONNREFUSED); channel_->ModifyActivityQueryCount(-1); diff --git a/src/env-inl.h b/src/env-inl.h index c361c8fa63..15b5010deb 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -746,13 +746,9 @@ inline void IsolateData::set_options( } template <typename Fn> -void Environment::CreateImmediate(Fn&& cb, - v8::Local<v8::Object> keep_alive, - bool ref) { +void Environment::CreateImmediate(Fn&& cb, bool ref) { auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>( - std::move(cb), - v8::Global<v8::Object>(isolate(), keep_alive), - ref); + std::move(cb), ref); NativeImmediateCallback* prev_tail = native_immediate_callbacks_tail_; native_immediate_callbacks_tail_ = callback.get(); @@ -765,8 +761,8 @@ void Environment::CreateImmediate(Fn&& cb, } template <typename Fn> -void Environment::SetImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) { - CreateImmediate(std::move(cb), keep_alive, true); +void Environment::SetImmediate(Fn&& cb) { + CreateImmediate(std::move(cb), true); if (immediate_info()->ref_count() == 0) ToggleImmediateRef(true); @@ -774,8 +770,8 @@ void Environment::SetImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) { } template <typename Fn> -void Environment::SetUnrefImmediate(Fn&& cb, v8::Local<v8::Object> keep_alive) { - CreateImmediate(std::move(cb), keep_alive, false); +void Environment::SetUnrefImmediate(Fn&& cb) { + CreateImmediate(std::move(cb), false); } Environment::NativeImmediateCallback::NativeImmediateCallback(bool refed) @@ -797,10 +793,9 @@ void Environment::NativeImmediateCallback::set_next( template <typename Fn> Environment::NativeImmediateCallbackImpl<Fn>::NativeImmediateCallbackImpl( - Fn&& callback, v8::Global<v8::Object>&& keep_alive, bool refed) + Fn&& callback, bool refed) : NativeImmediateCallback(refed), - callback_(std::move(callback)), - keep_alive_(std::move(keep_alive)) {} + callback_(std::move(callback)) {} template <typename Fn> void Environment::NativeImmediateCallbackImpl<Fn>::Call(Environment* env) { @@ -1183,13 +1183,9 @@ class Environment : public MemoryRetainer { // cb will be called as cb(env) on the next event loop iteration. // keep_alive will be kept alive between now and after the callback has run. template <typename Fn> - inline void SetImmediate(Fn&& cb, - v8::Local<v8::Object> keep_alive = - v8::Local<v8::Object>()); + inline void SetImmediate(Fn&& cb); template <typename Fn> - inline void SetUnrefImmediate(Fn&& cb, - v8::Local<v8::Object> keep_alive = - v8::Local<v8::Object>()); + inline void SetUnrefImmediate(Fn&& cb); // This needs to be available for the JS-land setImmediate(). void ToggleImmediateRef(bool ref); @@ -1260,9 +1256,7 @@ class Environment : public MemoryRetainer { private: template <typename Fn> - inline void CreateImmediate(Fn&& cb, - v8::Local<v8::Object> keep_alive, - bool ref); + inline void CreateImmediate(Fn&& cb, bool ref); inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>), const char* errmsg); @@ -1410,14 +1404,11 @@ class Environment : public MemoryRetainer { template <typename Fn> class NativeImmediateCallbackImpl final : public NativeImmediateCallback { public: - NativeImmediateCallbackImpl(Fn&& callback, - v8::Global<v8::Object>&& keep_alive, - bool refed); + NativeImmediateCallbackImpl(Fn&& callback, bool refed); void Call(Environment* env) override; private: Fn callback_; - v8::Global<v8::Object> keep_alive_; }; std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_; diff --git a/src/node_http2.cc b/src/node_http2.cc index 9eea9c257f..7170907ced 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1582,7 +1582,8 @@ void Http2Session::MaybeScheduleWrite() { HandleScope handle_scope(env()->isolate()); Debug(this, "scheduling write"); flags_ |= SESSION_STATE_WRITE_SCHEDULED; - env()->SetImmediate([this](Environment* env) { + BaseObjectPtr<Http2Session> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { if (session_ == nullptr || !(flags_ & SESSION_STATE_WRITE_SCHEDULED)) { // This can happen e.g. when a stream was reset before this turn // of the event loop, in which case SendPendingData() is called early, @@ -1595,7 +1596,7 @@ void Http2Session::MaybeScheduleWrite() { HandleScope handle_scope(env->isolate()); InternalCallbackScope callback_scope(this); SendPendingData(); - }, object()); + }); } } @@ -2043,7 +2044,8 @@ void Http2Stream::Destroy() { // Wait until the start of the next loop to delete because there // may still be some pending operations queued for this stream. - env()->SetImmediate([this](Environment* env) { + BaseObjectPtr<Http2Stream> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { // Free any remaining outgoing data chunks here. This should be done // here because it's possible for destroy to have been called while // we still have queued outbound writes. @@ -2057,9 +2059,11 @@ void Http2Stream::Destroy() { // We can destroy the stream now if there are no writes for it // already on the socket. Otherwise, we'll wait for the garbage collector // to take care of cleaning up. - if (session() == nullptr || !session()->HasWritesOnSocketForStream(this)) - delete this; - }, object()); + if (session() == nullptr || !session()->HasWritesOnSocketForStream(this)) { + // Delete once strong_ref goes out of scope. + Detach(); + } + }); statistics_.end_time = uv_hrtime(); session_->statistics_.stream_average_duration = diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index 832a20d324..6e339378ce 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -71,6 +71,7 @@ void StreamPipe::Unpipe() { // Delay the JS-facing part with SetImmediate, because this might be from // inside the garbage collector, so we can’t run JS here. HandleScope handle_scope(env()->isolate()); + BaseObjectPtr<StreamPipe> strong_ref{this}; env()->SetImmediate([this](Environment* env) { HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -105,7 +106,7 @@ void StreamPipe::Unpipe() { .IsNothing()) { return; } - }, object()); + }); } uv_buf_t StreamPipe::ReadableListener::OnStreamAlloc(size_t suggested_size) { diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 42b9469e38..4ec6dda6df 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -316,9 +316,10 @@ void TLSWrap::EncOut() { // its not clear if it is always correct. Not calling Done() could block // data flow, so for now continue to call Done(), just do it in the next // tick. - env()->SetImmediate([this](Environment* env) { + BaseObjectPtr<TLSWrap> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { InvokeQueued(0); - }, object()); + }); } } return; @@ -349,9 +350,10 @@ void TLSWrap::EncOut() { HandleScope handle_scope(env()->isolate()); // Simulate asynchronous finishing, TLS cannot handle this at the moment. - env()->SetImmediate([this](Environment* env) { + BaseObjectPtr<TLSWrap> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { OnStreamAfterWrite(nullptr, 0); - }, object()); + }); } } @@ -718,9 +720,10 @@ int TLSWrap::DoWrite(WriteWrap* w, StreamWriteResult res = underlying_stream()->Write(bufs, count, send_handle); if (!res.async) { - env()->SetImmediate([this](Environment* env) { + BaseObjectPtr<TLSWrap> strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { OnStreamAfterWrite(current_empty_write_, 0); - }, object()); + }); } return 0; } |