Skip to content

src: do not use std::function for OnScopeLeave #30134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void InternalCallbackScope::Close() {

if (!env_->can_call_into_js()) return;

OnScopeLeave weakref_cleanup([&]() { env_->RunWeakRefCleanup(); });
auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });

if (!tick_info->has_tick_scheduled()) {
MicrotasksScope::PerformCheckpoint(env_->isolate());
Expand Down
2 changes: 1 addition & 1 deletion src/large_pages/node_large_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ MoveTextRegionToLargePages(const text_region& r) {
PrintSystemError(errno);
return -1;
}
OnScopeLeave munmap_on_return([nmem, size]() {
auto munmap_on_return = OnScopeLeave([nmem, size]() {
if (-1 == munmap(nmem, size)) PrintSystemError(errno);
});

Expand Down
2 changes: 1 addition & 1 deletion src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void* wrapped_dlopen(const char* filename, int flags) {
Mutex::ScopedLock lock(dlhandles_mutex);

uv_fs_t req;
OnScopeLeave cleanup([&]() { uv_fs_req_cleanup(&req); });
auto cleanup = OnScopeLeave([&]() { uv_fs_req_cleanup(&req); });
int rc = uv_fs_stat(nullptr, &req, filename, nullptr);

if (rc != 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/node_env_var.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
uv_env_item_t* items;
int count;

OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
CHECK_EQ(uv_os_environ(&items, &count), 0);

MaybeStackBuffer<Local<Value>, 256> env_v(count);
Expand Down
2 changes: 1 addition & 1 deletion src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ class Parser : public AsyncWrap, public StreamListener {
// Once we’re done here, either indicate that the HTTP parser buffer
// is free for re-use, or free() the data if it didn’t come from there
// in the first place.
OnScopeLeave on_scope_leave([&]() {
auto on_scope_leave = OnScopeLeave([&]() {
if (buf.base == env()->http_parser_buffer())
env()->set_http_parser_buffer_in_use(false);
else
Expand Down
2 changes: 1 addition & 1 deletion src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class ConverterObject : public BaseObject, Converter {
result.AllocateSufficientStorage(limit);

UBool flush = (flags & CONVERTER_FLAGS_FLUSH) == CONVERTER_FLAGS_FLUSH;
OnScopeLeave cleanup([&]() {
auto cleanup = OnScopeLeave([&]() {
if (flush) {
// Reset the converter state.
converter->bomSeen_ = false;
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
per_process::cli_options->per_isolate = env->isolate_data()->options();
auto original_per_env = per_process::cli_options->per_isolate->per_env;
per_process::cli_options->per_isolate->per_env = env->options();
OnScopeLeave on_scope_leave([&]() {
auto on_scope_leave = OnScopeLeave([&]() {
per_process::cli_options->per_isolate->per_env = original_per_env;
per_process::cli_options->per_isolate = original_per_isolate;
});
Expand Down
2 changes: 1 addition & 1 deletion src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
return args.GetReturnValue().SetUndefined();
}

OnScopeLeave free_passwd([&]() { uv_os_free_passwd(&pwd); });
auto free_passwd = OnScopeLeave([&]() { uv_os_free_passwd(&pwd); });

Local<Value> error;

Expand Down
2 changes: 1 addition & 1 deletion src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static void DebugProcess(const FunctionCallbackInfo<Value>& args) {
LPTHREAD_START_ROUTINE* handler = nullptr;
DWORD pid = 0;

OnScopeLeave cleanup([&]() {
auto cleanup = OnScopeLeave([&]() {
if (process != nullptr) CloseHandle(process);
if (thread != nullptr) CloseHandle(thread);
if (handler != nullptr) UnmapViewOfFile(handler);
Expand Down
2 changes: 1 addition & 1 deletion src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ void Worker::Run() {
#endif

DeleteFnPtr<Environment, FreeEnvironment> env_;
OnScopeLeave cleanup_env([&]() {
auto cleanup_env = OnScopeLeave([&]() {
if (!env_) return;
env_->set_can_call_into_js(false);
Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_,
Expand Down
2 changes: 1 addition & 1 deletion src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
// v8 land!
void AfterThreadPoolWork(int status) override {
AllocScope alloc_scope(this);
OnScopeLeave on_scope_leave([&]() { Unref(); });
auto on_scope_leave = OnScopeLeave([&]() { Unref(); });

write_in_progress_ = false;

Expand Down
45 changes: 34 additions & 11 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
#include <unordered_map>
#include <utility>

#ifdef __GNUC__
#define MUST_USE_RESULT __attribute__((warn_unused_result))
#else
#define MUST_USE_RESULT
#endif

namespace node {

// Maybe remove kPathSeparator when cpp17 is ready
Expand Down Expand Up @@ -494,14 +500,37 @@ class BufferValue : public MaybeStackBuffer<char> {
// silence a compiler warning about that.
template <typename T> inline void USE(T&&) {}

// Run a function when exiting the current scope.
struct OnScopeLeave {
std::function<void()> fn_;
template <typename Fn>
struct OnScopeLeaveImpl {
Fn fn_;
bool active_;

explicit OnScopeLeaveImpl(Fn&& fn) : fn_(std::move(fn)), active_(true) {}
~OnScopeLeaveImpl() { if (active_) fn_(); }

explicit OnScopeLeave(std::function<void()> fn) : fn_(std::move(fn)) {}
~OnScopeLeave() { fn_(); }
OnScopeLeaveImpl(const OnScopeLeaveImpl& other) = delete;
OnScopeLeaveImpl& operator=(const OnScopeLeaveImpl& other) = delete;
OnScopeLeaveImpl(OnScopeLeaveImpl&& other)
: fn_(std::move(other.fn_)), active_(other.active_) {
other.active_ = false;
}
OnScopeLeaveImpl& operator=(OnScopeLeaveImpl&& other) {
if (this == &other) return *this;
this->~OnScopeLeave();
new (this)OnScopeLeaveImpl(std::move(other));
return *this;
}
};

// Run a function when exiting the current scope. Used like this:
// auto on_scope_leave = OnScopeLeave([&] {
// // ... run some code ...
// });
template <typename Fn>
inline MUST_USE_RESULT OnScopeLeaveImpl<Fn> OnScopeLeave(Fn&& fn) {
return OnScopeLeaveImpl<Fn>{std::move(fn)};
}

// Simple RAII wrapper for contiguous data that uses malloc()/free().
template <typename T>
struct MallocedBuffer {
Expand Down Expand Up @@ -679,12 +708,6 @@ constexpr T RoundUp(T a, T b) {
return a % b != 0 ? a + b - (a % b) : a;
}

#ifdef __GNUC__
#define MUST_USE_RESULT __attribute__((warn_unused_result))
#else
#define MUST_USE_RESULT
#endif

class SlicedArguments : public MaybeStackBuffer<v8::Local<v8::Value>> {
public:
inline explicit SlicedArguments(
Expand Down