Skip to content

Commit 099f18e

Browse files
addaleaxcodebytere
authored andcommitted
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 <[email protected]>
1 parent 9d1e577 commit 099f18e

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
@@ -727,9 +727,9 @@ void Environment::SetUnrefImmediate(Fn&& cb) {
727727
}
728728

729729
template <typename Fn>
730-
void Environment::SetImmediateThreadsafe(Fn&& cb) {
730+
void Environment::SetImmediateThreadsafe(Fn&& cb, bool refed) {
731731
auto callback =
732-
native_immediates_threadsafe_.CreateCallback(std::move(cb), false);
732+
native_immediates_threadsafe_.CreateCallback(std::move(cb), refed);
733733
{
734734
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
735735
native_immediates_threadsafe_.Push(std::move(callback));

src/env.cc

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

744-
// It is safe to check .size() first, because there is a causal relationship
745-
// between pushes to the threadsafe and this function being called.
746-
// For the common case, it's worth checking the size first before establishing
747-
// a mutex lock.
748-
if (native_immediates_threadsafe_.size() > 0) {
749-
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
750-
native_immediates_.ConcatMove(std::move(native_immediates_threadsafe_));
751-
}
752-
753-
auto drain_list = [&]() {
744+
auto drain_list = [&](NativeImmediateQueue* queue) {
754745
TryCatchScope try_catch(this);
755746
DebugSealHandleScope seal_handle_scope(isolate());
756-
while (auto head = native_immediates_.Shift()) {
747+
while (auto head = queue->Shift()) {
757748
if (head->is_refed())
758749
ref_count++;
759750

@@ -771,12 +762,26 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
771762
}
772763
return false;
773764
};
774-
while (drain_list()) {}
765+
while (drain_list(&native_immediates_)) {}
775766

776767
immediate_info()->ref_count_dec(ref_count);
777768

778769
if (immediate_info()->ref_count() == 0)
779770
ToggleImmediateRef(false);
771+
772+
// It is safe to check .size() first, because there is a causal relationship
773+
// between pushes to the threadsafe immediate list and this function being
774+
// called. For the common case, it's worth checking the size first before
775+
// establishing a mutex lock.
776+
// This is intentionally placed after the `ref_count` handling, because when
777+
// refed threadsafe immediates are created, they are not counted towards the
778+
// count in immediate_info() either.
779+
NativeImmediateQueue threadsafe_immediates;
780+
if (native_immediates_threadsafe_.size() > 0) {
781+
Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
782+
threadsafe_immediates.ConcatMove(std::move(native_immediates_threadsafe_));
783+
}
784+
while (drain_list(&threadsafe_immediates)) {}
780785
}
781786

782787
void Environment::RequestInterruptFromV8() {

src/env.h

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

src/node_worker.cc

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

0 commit comments

Comments
 (0)