FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

v8: make v8.writeHeapSnapshot() error codes consistent · nodejs/node@0187bc5 · GitHub

/ node Public

Commit 0187bc5

Browse files
RaisinTen
authored
v8: make v8.writeHeapSnapshot() error codes consistent
This change makes the error codes returned by v8.writeHeapSnapshot() consistent across all platforms by using the libuv APIs instead of fopen(), fwrite() and fclose(). This also starts reporting potential errors that might happen during the write operations. Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #42577 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 818284b commit 0187bc5

4 files changed

Lines changed: 77 additions & 26 deletions

File tree

‎doc/api/v8.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,9 @@ changes:
278278
- version: REPLACEME
279279
pr-url: https://github.com/nodejs/node/pull/41373
280280
description: An exception will now be thrown if the file could not be written.
281+
- version: REPLACEME
282+
pr-url: https://github.com/nodejs/node/pull/42577
283+
description: Make the returned error codes consistent across all platforms.
281284
-->
282285

283286
* `filename` {string} The file path where the V8 heap snapshot is to be

‎src/heap_utils.cc‎

Lines changed: 72 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,17 @@
55
#include "stream_base-inl.h"
66
#include "util-inl.h"
77

8+
// Copied from https://github.com/nodejs/node/blob/b07dc4d19fdbc15b4f76557dc45b3ce3a43ad0c3/src/util.cc#L36-L41.
9+
#ifdef _WIN32
10+
#include <io.h> // _S_IREAD _S_IWRITE
11+
#ifndef S_IRUSR
12+
#define S_IRUSR _S_IREAD
13+
#endif // S_IRUSR
14+
#ifndef S_IWUSR
15+
#define S_IWUSR _S_IWRITE
16+
#endif // S_IWUSR
17+
#endif
18+
819
using v8::Array;
920
using v8::Boolean;
1021
using v8::Context;
@@ -16,8 +27,11 @@ using v8::Global;
1627
using v8::HandleScope;
1728
using v8::HeapSnapshot;
1829
using v8::Isolate;
30+
using v8::JustVoid;
1931
using v8::Local;
32+
using v8::Maybe;
2033
using v8::MaybeLocal;
34+
using v8::Nothing;
2135
using v8::Number;
2236
using v8::Object;
2337
using v8::ObjectTemplate;
@@ -206,26 +220,44 @@ void BuildEmbedderGraph(const FunctionCallbackInfo<Value>& args) {
206220
namespace {
207221
class FileOutputStream : public v8::OutputStream {
208222
public:
209-
explicit FileOutputStream(FILE* stream) : stream_(stream) {}
223+
FileOutputStream(const int fd, uv_fs_t* req) : fd_(fd), req_(req) {}
210224

211225
int GetChunkSize() override {
212226
return 65536; // big chunks == faster
213227
}
214228

215229
void EndOfStream() override {}
216230

217-
WriteResult WriteAsciiChunk(char* data, int size) override {
218-
const size_t len = static_cast<size_t>(size);
219-
size_t off = 0;
220-
221-
while (off < len && !feof(stream_) && !ferror(stream_))
222-
off += fwrite(data + off, 1, len - off, stream_);
223-
224-
return off == len ? kContinue : kAbort;
231+
WriteResult WriteAsciiChunk(char* data, const int size) override {
232+
DCHECK_EQ(status_, 0);
233+
int offset = 0;
234+
while (offset < size) {
235+
const uv_buf_t buf = uv_buf_init(data + offset, size - offset);
236+
const int num_bytes_written = uv_fs_write(nullptr,
237+
req_,
238+
fd_,
239+
&buf,
240+
1,
241+
-1,
242+
nullptr);
243+
uv_fs_req_cleanup(req_);
244+
if (num_bytes_written < 0) {
245+
status_ = num_bytes_written;
246+
return kAbort;
247+
}
248+
DCHECK_LE(num_bytes_written, buf.len);
249+
offset += num_bytes_written;
250+
}
251+
DCHECK_EQ(offset, size);
252+
return kContinue;
225253
}
226254

255+
int status() const { return status_; }
256+
227257
private:
228-
FILE* stream_;
258+
const int fd_;
259+
uv_fs_t* req_;
260+
int status_ = 0;
229261
};
230262

231263
class HeapSnapshotStream : public AsyncWrap,
@@ -316,19 +348,37 @@ inline void TakeSnapshot(Environment* env, v8::OutputStream* out) {
316348

317349
} // namespace
318350

319-
bool WriteSnapshot(Environment* env, const char* filename) {
320-
FILE* fp = fopen(filename, "w");
321-
if (fp == nullptr) {
322-
env->ThrowErrnoException(errno, "open");
323-
return false;
351+
Maybe<void> WriteSnapshot(Environment* env, const char* filename) {
352+
uv_fs_t req;
353+
int err;
354+
355+
const int fd = uv_fs_open(nullptr,
356+
&req,
357+
filename,
358+
O_WRONLY | O_CREAT | O_TRUNC,
359+
S_IWUSR | S_IRUSR,
360+
nullptr);
361+
uv_fs_req_cleanup(&req);
362+
if ((err = fd) < 0) {
363+
env->ThrowUVException(err, "open", nullptr, filename);
364+
return Nothing<void>();
324365
}
325-
FileOutputStream stream(fp);
366+
367+
FileOutputStream stream(fd, &req);
326368
TakeSnapshot(env, &stream);
327-
if (fclose(fp) == EOF) {
328-
env->ThrowErrnoException(errno, "close");
329-
return false;
369+
if ((err = stream.status()) < 0) {
370+
env->ThrowUVException(err, "write", nullptr, filename);
371+
return Nothing<void>();
330372
}
331-
return true;
373+
374+
err = uv_fs_close(nullptr, &req, fd, nullptr);
375+
uv_fs_req_cleanup(&req);
376+
if (err < 0) {
377+
env->ThrowUVException(err, "close", nullptr, filename);
378+
return Nothing<void>();
379+
}
380+
381+
return JustVoid();
332382
}
333383

334384
void DeleteHeapSnapshot(const HeapSnapshot* snapshot) {
@@ -379,7 +429,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
379429

380430
if (filename_v->IsUndefined()) {
381431
DiagnosticFilename name(env, "Heap", "heapsnapshot");
382-
if (!WriteSnapshot(env, *name))
432+
if (WriteSnapshot(env, *name).IsNothing())
383433
return;
384434
if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) {
385435
args.GetReturnValue().Set(filename_v);
@@ -389,7 +439,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
389439

390440
BufferValue path(isolate, filename_v);
391441
CHECK_NOT_NULL(*path);
392-
if (!WriteSnapshot(env, *path))
442+
if (WriteSnapshot(env, *path).IsNothing())
393443
return;
394444
return args.GetReturnValue().Set(filename_v);
395445
}

‎src/node_internals.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ class DiagnosticFilename {
379379
};
380380

381381
namespace heap {
382-
bool WriteSnapshot(Environment* env, const char* filename);
382+
v8::Maybe<void> WriteSnapshot(Environment* env, const char* filename);
383383
}
384384

385385
class TraceEventScope {

‎test/sequential/test-heapdump.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ process.chdir(tmpdir.path);
3131
writeHeapSnapshot(directory);
3232
}, (e) => {
3333
assert.ok(e, 'writeHeapSnapshot should error');
34-
// TODO(RaisinTen): This should throw EISDIR on Windows too.
35-
assert.strictEqual(e.code,
36-
process.platform === 'win32' ? 'EACCES' : 'EISDIR');
34+
assert.strictEqual(e.code, 'EISDIR');
3735
assert.strictEqual(e.syscall, 'open');
3836
return true;
3937
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL