Skip to content

Commit b924910

Browse files
addaleaxcodebytere
authored andcommittedJun 9, 2020
src: distinguish refed/unrefed threadsafe Immediates
In some situations, it can be useful to use threadsafe callbacks on an `Environment` to perform cleanup operations that should run even when the process would otherwise be ending. PR-URL: #33320 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b4d9034 commit b924910

File tree

4 files changed

+21
-16
lines changed

4 files changed

+21
-16
lines changed
 

‎src/env-inl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -752,9 +752,9 @@ void Environment::SetUnrefImmediate(Fn&& cb) {
752752
}
753753

754754
template <typename Fn>
755-
void Environment::SetImmediateThreadsafe(Fn&& cb) {
755+
void Environment::SetImmediateThreadsafe(Fn&& cb, bool refed) {
756756
auto callback =
757-
native_immediates_threadsafe_.CreateCallback(std::move(cb), false);
757+
native_immediates_threadsafe_.CreateCallback(std::move(cb), refed);
758758
{
759759
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
760760
native_immediates_threadsafe_.Push(std::move(callback));

‎src/env.cc

+17-12
Original file line numberDiff line numberDiff line change
@@ -704,19 +704,10 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
704704
// exceptions, so we do not need to handle that.
705705
RunAndClearInterrupts();
706706

707-
// It is safe to check .size() first, because there is a causal relationship
708-
// between pushes to the threadsafe and this function being called.
709-
// For the common case, it's worth checking the size first before establishing
710-
// a mutex lock.
711-
if (native_immediates_threadsafe_.size() > 0) {
712-
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
713-
native_immediates_.ConcatMove(std::move(native_immediates_threadsafe_));
714-
}
715-
716-
auto drain_list = [&]() {
707+
auto drain_list = [&](NativeImmediateQueue* queue) {
717708
TryCatchScope try_catch(this);
718709
DebugSealHandleScope seal_handle_scope(isolate());
719-
while (auto head = native_immediates_.Shift()) {
710+
while (auto head = queue->Shift()) {
720711
if (head->is_refed())
721712
ref_count++;
722713

@@ -734,12 +725,26 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
734725
}
735726
return false;
736727
};
737-
while (drain_list()) {}
728+
while (drain_list(&native_immediates_)) {}
738729

739730
immediate_info()->ref_count_dec(ref_count);
740731

741732
if (immediate_info()->ref_count() == 0)
742733
ToggleImmediateRef(false);
734+
735+
// It is safe to check .size() first, because there is a causal relationship
736+
// between pushes to the threadsafe immediate list and this function being
737+
// called. For the common case, it's worth checking the size first before
738+
// establishing a mutex lock.
739+
// This is intentionally placed after the `ref_count` handling, because when
740+
// refed threadsafe immediates are created, they are not counted towards the
741+
// count in immediate_info() either.
742+
NativeImmediateQueue threadsafe_immediates;
743+
if (native_immediates_threadsafe_.size() > 0) {
744+
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
745+
threadsafe_immediates.ConcatMove(std::move(native_immediates_threadsafe_));
746+
}
747+
while (drain_list(&threadsafe_immediates)) {}
743748
}
744749

745750
void Environment::RequestInterruptFromV8() {

‎src/env.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -1185,7 +1185,7 @@ class Environment : public MemoryRetainer {
11851185
inline void SetUnrefImmediate(Fn&& cb);
11861186
template <typename Fn>
11871187
// This behaves like SetImmediate() but can be called from any thread.
1188-
inline void SetImmediateThreadsafe(Fn&& cb);
1188+
inline void SetImmediateThreadsafe(Fn&& cb, bool refed = true);
11891189
// This behaves like V8's Isolate::RequestInterrupt(), but also accounts for
11901190
// the event loop (i.e. combines the V8 function with SetImmediate()).
11911191
// The passed callback may not throw exceptions.

‎src/node_worker.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ void Worker::TakeHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
764764
env, std::move(snapshot));
765765
Local<Value> args[] = { stream->object() };
766766
taker->MakeCallback(env->ondone_string(), arraysize(args), args);
767-
});
767+
}, /* refed */ false);
768768
});
769769
args.GetReturnValue().Set(scheduled ? taker->object() : Local<Object>());
770770
}

0 commit comments

Comments
 (0)
Please sign in to comment.