Skip to content

fs: encapsulate FSReqWrap more #18112

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.

Sign up for GitHub

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 1 commit into from
Closed

fs: encapsulate FSReqWrap more #18112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 47 additions & 73 deletions src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ using v8::MaybeLocal;
using v8::Number;
using v8::Object;
using v8::String;
using v8::Undefined;
using v8::Value;

#ifndef MIN
Expand All @@ -102,32 +103,16 @@ using v8::Value;

#define GET_OFFSET(a) ((a)->IsNumber() ? (a)->IntegerValue() : -1)

FSReqWrap* FSReqWrap::New(Environment* env,
Local<Object> req,
const char* syscall,
const char* data,
enum encoding encoding,
Ownership ownership) {
const bool copy = (data != nullptr && ownership == COPY);
const size_t size = copy ? 1 + strlen(data) : 0;
FSReqWrap* that;
char* const storage = new char[sizeof(*that) + size];
that = new(storage) FSReqWrap(env, req, syscall, data, encoding);
if (copy)
that->data_ = static_cast<char*>(memcpy(that->inline_data(), data, size));
return that;
void FSReqWrap::Reject(Local<Value> reject) {
MakeCallback(env()->oncomplete_string(), 1, &reject);
}


void FSReqWrap::Dispose() {
this->~FSReqWrap();
delete[] reinterpret_cast<char*>(this);
void FSReqWrap::FillStatsArray(const uv_stat_t* stat) {
node::FillStatsArray(env()->fs_stats_field_array(), stat);
}


void FSReqWrap::Reject(Local<Value> reject) {
Local<Value> argv[1] { reject };
MakeCallback(env()->oncomplete_string(), arraysize(argv), argv);
void FSReqWrap::ResolveStat() {
Resolve(Undefined(env()->isolate()));
}

void FSReqWrap::Resolve(Local<Value> value) {
Expand All @@ -138,9 +123,26 @@ void FSReqWrap::Resolve(Local<Value> value) {
MakeCallback(env()->oncomplete_string(), arraysize(argv), argv);
}

void FSReqWrap::Init(const char* syscall,
const char* data,
size_t len,
enum encoding encoding) {
syscall_ = syscall;
encoding_ = encoding;

if (data != nullptr) {
CHECK_EQ(data_, nullptr);
buffer_.AllocateSufficientStorage(len + 1);
buffer_.SetLengthAndZeroTerminate(len);
memcpy(*buffer_, data, len);
data_ = *buffer_;
}
}

void NewFSReqWrap(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
ClearWrap(args.This());
Environment* env = Environment::GetCurrent(args.GetIsolate());
new FSReqWrap(env, args.This());
}


Expand All @@ -150,12 +152,11 @@ FSReqAfterScope::FSReqAfterScope(FSReqWrap* wrap, uv_fs_t* req)
handle_scope_(wrap->env()->isolate()),
context_scope_(wrap->env()->context()) {
CHECK_EQ(wrap_->req(), req);
wrap_->ReleaseEarly(); // Free memory that's no longer used now.
}

FSReqAfterScope::~FSReqAfterScope() {
uv_fs_req_cleanup(wrap_->req());
wrap_->Dispose();
delete wrap_;
}

// TODO(joyeecheung): create a normal context object, and
Expand Down Expand Up @@ -195,12 +196,10 @@ void AfterNoArgs(uv_fs_t* req) {
void AfterStat(uv_fs_t* req) {
FSReqWrap* req_wrap = static_cast<FSReqWrap*>(req->data);
FSReqAfterScope after(req_wrap, req);
Environment* env = req_wrap->env();

if (after.Proceed()) {
FillStatsArray(env->fs_stats_field_array(),
static_cast<const uv_stat_t*>(req->ptr));
req_wrap->Resolve(Undefined(req_wrap->env()->isolate()));
req_wrap->FillStatsArray(&req->statbuf);
req_wrap->ResolveStat();
}
}

Expand All @@ -222,7 +221,7 @@ void AfterStringPath(uv_fs_t* req) {
if (after.Proceed()) {
link = StringBytes::Encode(req_wrap->env()->isolate(),
static_cast<const char*>(req->path),
req_wrap->encoding_,
req_wrap->encoding(),
&error);
if (link.IsEmpty())
req_wrap->Reject(error);
Expand All @@ -241,7 +240,7 @@ void AfterStringPtr(uv_fs_t* req) {
if (after.Proceed()) {
link = StringBytes::Encode(req_wrap->env()->isolate(),
static_cast<const char*>(req->ptr),
req_wrap->encoding_,
req_wrap->encoding(),
&error);
if (link.IsEmpty())
req_wrap->Reject(error);
Expand Down Expand Up @@ -278,7 +277,7 @@ void AfterScanDir(uv_fs_t* req) {
MaybeLocal<Value> filename =
StringBytes::Encode(env->isolate(),
ent.name,
req_wrap->encoding_,
req_wrap->encoding(),
&error);
if (filename.IsEmpty())
return req_wrap->Reject(error);
Expand Down Expand Up @@ -317,12 +316,12 @@ class fs_req_wrap {
template <typename Func, typename... Args>
inline FSReqWrap* AsyncDestCall(Environment* env,
const FunctionCallbackInfo<Value>& args,
const char* syscall, const char* dest,
enum encoding enc, FSReqWrap::Ownership ownership,
uv_fs_cb after, Func fn, Args... fn_args) {
const char* syscall, const char* dest, size_t len,
enum encoding enc, uv_fs_cb after, Func fn, Args... fn_args) {
Local<Object> req = args[args.Length() - 1].As<Object>();
FSReqWrap* req_wrap = FSReqWrap::New(env, req,
syscall, dest, enc, ownership);
FSReqWrap* req_wrap = Unwrap<FSReqWrap>(req);
CHECK_NE(req_wrap, nullptr);
req_wrap->Init(syscall, dest, len, enc);
int err = fn(env->event_loop(), req_wrap->req(), fn_args..., after);
req_wrap->Dispatched();
if (err < 0) {
Expand All @@ -339,38 +338,16 @@ inline FSReqWrap* AsyncDestCall(Environment* env,
return req_wrap;
}

// Defaults to COPY ownership.
template <typename Func, typename... Args>
inline FSReqWrap* AsyncDestCall(Environment* env,
const FunctionCallbackInfo<Value>& args,
const char* syscall, const char* dest, enum encoding enc,
uv_fs_cb after, Func fn, Args... fn_args) {
return AsyncDestCall(env, args,
syscall, dest, enc, FSReqWrap::COPY,
after, fn, fn_args...);
}

template <typename Func, typename... Args>
inline FSReqWrap* AsyncCall(Environment* env,
const FunctionCallbackInfo<Value>& args,
const char* syscall, enum encoding enc, FSReqWrap::Ownership ownership,
const char* syscall, enum encoding enc,
uv_fs_cb after, Func fn, Args... fn_args) {
return AsyncDestCall(env, args,
syscall, nullptr, enc, ownership,
syscall, nullptr, 0, enc,
after, fn, fn_args...);
}

// Defaults to COPY ownership.
template <typename Func, typename... Args>
inline FSReqWrap* AsyncCall(Environment* env,
const FunctionCallbackInfo<Value>& args,
const char* syscall, enum encoding enc,
uv_fs_cb after, Func fn, Args... fn_args) {
return AsyncCall(env, args,
syscall, enc, FSReqWrap::COPY,
after, fn, fn_args...);
}

// Template counterpart of SYNC_CALL, except that it only puts
// the error number and the syscall in the context instead of
// creating an error in the C++ land.
Expand Down Expand Up @@ -623,8 +600,8 @@ static void Symlink(const FunctionCallbackInfo<Value>& args) {

if (args[3]->IsObject()) { // symlink(target, path, flags, req)
CHECK_EQ(args.Length(), 4);
AsyncDestCall(env, args, "symlink", *path, UTF8, AfterNoArgs,
uv_fs_symlink, *target, *path, flags);
AsyncDestCall(env, args, "symlink", *path, path.length(), UTF8,
AfterNoArgs, uv_fs_symlink, *target, *path, flags);
} else { // symlink(target, path, flags)
SYNC_DEST_CALL(symlink, *target, *path, *target, *path, flags)
}
Expand All @@ -643,8 +620,8 @@ static void Link(const FunctionCallbackInfo<Value>& args) {

if (args[2]->IsObject()) { // link(src, dest, req)
CHECK_EQ(args.Length(), 3);
AsyncDestCall(env, args, "link", *dest, UTF8, AfterNoArgs,
uv_fs_link, *src, *dest);
AsyncDestCall(env, args, "link", *dest, dest.length(), UTF8,
AfterNoArgs, uv_fs_link, *src, *dest);
} else { // link(src, dest)
SYNC_DEST_CALL(link, *src, *dest, *src, *dest)
}
Expand Down Expand Up @@ -695,8 +672,8 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {

if (args[2]->IsObject()) {
CHECK_EQ(args.Length(), 3);
AsyncDestCall(env, args, "rename", *new_path, UTF8, AfterNoArgs,
uv_fs_rename, *old_path, *new_path);
AsyncDestCall(env, args, "rename", *new_path, new_path.length(),
UTF8, AfterNoArgs, uv_fs_rename, *old_path, *new_path);
} else {
SYNC_DEST_CALL(rename, *old_path, *new_path, *old_path, *new_path)
}
Expand Down Expand Up @@ -1041,7 +1018,6 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
char* buf = nullptr;
int64_t pos;
size_t len;
FSReqWrap::Ownership ownership = FSReqWrap::COPY;

// will assign buf and len if string was external
if (!StringBytes::GetExternalParts(string,
Expand All @@ -1053,16 +1029,14 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
// StorageSize may return too large a char, so correct the actual length
// by the write size
len = StringBytes::Write(env->isolate(), buf, len, args[1], enc);
ownership = FSReqWrap::MOVE;
}
pos = GET_OFFSET(args[2]);

uv_buf_t uvbuf = uv_buf_init(const_cast<char*>(buf), len);

if (args[4]->IsObject()) {
CHECK_EQ(args.Length(), 5);
AsyncCall(env, args,
"write", UTF8, ownership, AfterInteger,
AsyncCall(env, args, "write", UTF8, AfterInteger,
uv_fs_write, fd, &uvbuf, 1, pos);
} else {
// SYNC_CALL returns on error. Make sure to always free the memory.
Expand All @@ -1071,7 +1045,7 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
inline ~Delete() { delete[] pointer_; }
char* const pointer_;
};
Delete delete_on_return(ownership == FSReqWrap::MOVE ? buf : nullptr);
Delete delete_on_return(buf);
SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos)
return args.GetReturnValue().Set(SYNC_RESULT);
}
Expand Down Expand Up @@ -1373,7 +1347,7 @@ void InitFs(Local<Object> target,
Local<String> wrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "FSReqWrap");
fst->SetClassName(wrapString);
target->Set(wrapString, fst->GetFunction());
target->Set(context, wrapString, fst->GetFunction()).FromJust();
}

} // namespace fs
Expand Down
59 changes: 19 additions & 40 deletions src/node_file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,60 +17,39 @@ using v8::Value;

namespace fs {

class FSReqWrap: public ReqWrap<uv_fs_t> {
class FSReqWrap : public ReqWrap<uv_fs_t> {
public:
enum Ownership { COPY, MOVE };
FSReqWrap(Environment* env, Local<Object> req)
: ReqWrap(env, req, AsyncWrap::PROVIDER_FSREQWRAP) {
Wrap(object(), this);
}

inline static FSReqWrap* New(Environment* env,
Local<Object> req,
const char* syscall,
const char* data = nullptr,
enum encoding encoding = UTF8,
Ownership ownership = COPY);
virtual ~FSReqWrap() {
ClearWrap(object());
}

inline void Dispose();
void Init(const char* syscall,
const char* data = nullptr,
size_t len = 0,
enum encoding encoding = UTF8);

virtual void FillStatsArray(const uv_stat_t* stat);
virtual void Reject(Local<Value> reject);
virtual void Resolve(Local<Value> value);

void ReleaseEarly() {
if (data_ != inline_data()) {
delete[] data_;
data_ = nullptr;
}
}
virtual void ResolveStat();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these methods virtual?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the plan is to have FSReqPromise just extend from FSReqWrap and override those. That may change tho.


const char* syscall() const { return syscall_; }
const char* data() const { return data_; }
const enum encoding encoding_;
enum encoding encoding() const { return encoding_; }

size_t self_size() const override { return sizeof(*this); }

protected:
FSReqWrap(Environment* env,
Local<Object> req,
const char* syscall,
const char* data,
enum encoding encoding)
: ReqWrap(env, req, AsyncWrap::PROVIDER_FSREQWRAP),
encoding_(encoding),
syscall_(syscall),
data_(data) {
Wrap(object(), this);
}

virtual ~FSReqWrap() {
ReleaseEarly();
ClearWrap(object());
}

void* operator new(size_t size) = delete;
void* operator new(size_t size, char* storage) { return storage; }
char* inline_data() { return reinterpret_cast<char*>(this + 1); }

private:
enum encoding encoding_ = UTF8;
const char* syscall_;
const char* data_;

const char* data_ = nullptr;
MaybeStackBuffer<char> buffer_;

DISALLOW_COPY_AND_ASSIGN(FSReqWrap);
};
Expand Down
3 changes: 1 addition & 2 deletions test/sequential/test-async-wrap-getasyncid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check
const req = new FSReqWrap();
req.oncomplete = () => { };

testUninitialized(req, 'FSReqWrap');
binding.access(path.toNamespacedPath('../'), fs.F_OK, req);
testInitialized(req, 'FSReqWrap');
binding.access(path.toNamespacedPath('../'), fs.F_OK, req);

const StatWatcher = binding.StatWatcher;
testInitialized(new StatWatcher(), 'StatWatcher');
Expand Down