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

v8: make writeHeapSnapshot throw if fopen fails · nodejs/node@74b9baa · GitHub

/ node Public

Commit 74b9baa

Browse files
kyranet
authored
v8: make writeHeapSnapshot throw if fopen fails
If the file fails to be written (e.g. missing permissions, no space left on device, etc), `writeHeapSnapshot` will now throw an exception. This commit also adds error handling for the `fclose` call, returning false if a non-zero value was returned. Fixes: #41346 PR-URL: #41373 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 36035e0 commit 74b9baa

5 files changed

Lines changed: 32 additions & 10 deletions

File tree

‎doc/api/v8.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,10 @@ disk unless [`v8.stopCoverage()`][] is invoked before the process exits.
267267

268268
<!-- YAML
269269
added: v11.13.0
270+
changes:
271+
- version: REPLACEME
272+
pr-url: https://github.com/nodejs/node/pull/41373
273+
description: An exception will now be thrown if the file could not be written.
270274
-->
271275

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

‎src/env.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1643,7 +1643,7 @@ size_t Environment::NearHeapLimitCallback(void* data,
16431643
env->isolate()->RemoveNearHeapLimitCallback(NearHeapLimitCallback,
16441644
initial_heap_limit);
16451645

1646-
heap::WriteSnapshot(env->isolate(), filename.c_str());
1646+
heap::WriteSnapshot(env, filename.c_str());
16471647
env->heap_limit_snapshot_taken_ += 1;
16481648

16491649
// Don't take more snapshots than the number specified by

‎src/heap_utils.cc‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,21 +308,26 @@ class HeapSnapshotStream : public AsyncWrap,
308308
HeapSnapshotPointer snapshot_;
309309
};
310310

311-
inline void TakeSnapshot(Isolate* isolate, v8::OutputStream* out) {
311+
inline void TakeSnapshot(Environment* env, v8::OutputStream* out) {
312312
HeapSnapshotPointer snapshot {
313-
isolate->GetHeapProfiler()->TakeHeapSnapshot() };
313+
env->isolate()->GetHeapProfiler()->TakeHeapSnapshot() };
314314
snapshot->Serialize(out, HeapSnapshot::kJSON);
315315
}
316316

317317
} // namespace
318318

319-
bool WriteSnapshot(Isolate* isolate, const char* filename) {
319+
bool WriteSnapshot(Environment* env, const char* filename) {
320320
FILE* fp = fopen(filename, "w");
321-
if (fp == nullptr)
321+
if (fp == nullptr) {
322+
env->ThrowErrnoException(errno, "open");
322323
return false;
324+
}
323325
FileOutputStream stream(fp);
324-
TakeSnapshot(isolate, &stream);
325-
fclose(fp);
326+
TakeSnapshot(env, &stream);
327+
if (fclose(fp) == EOF) {
328+
env->ThrowErrnoException(errno, "close");
329+
return false;
330+
}
326331
return true;
327332
}
328333

@@ -374,7 +379,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
374379

375380
if (filename_v->IsUndefined()) {
376381
DiagnosticFilename name(env, "Heap", "heapsnapshot");
377-
if (!WriteSnapshot(isolate, *name))
382+
if (!WriteSnapshot(env, *name))
378383
return;
379384
if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) {
380385
args.GetReturnValue().Set(filename_v);
@@ -384,7 +389,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
384389

385390
BufferValue path(isolate, filename_v);
386391
CHECK_NOT_NULL(*path);
387-
if (!WriteSnapshot(isolate, *path))
392+
if (!WriteSnapshot(env, *path))
388393
return;
389394
return args.GetReturnValue().Set(filename_v);
390395
}

‎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(v8::Isolate* isolate, const char* filename);
382+
bool WriteSnapshot(Environment* env, const char* filename);
383383
}
384384

385385
class TraceEventScope {

‎test/sequential/test-heapdump.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ process.chdir(tmpdir.path);
2424
fs.accessSync(heapdump);
2525
}
2626

27+
{
28+
const readonlyFile = 'ro';
29+
fs.writeFileSync(readonlyFile, Buffer.alloc(0), { mode: 0o444 });
30+
assert.throws(() => {
31+
writeHeapSnapshot(readonlyFile);
32+
}, (e) => {
33+
assert.ok(e, 'writeHeapSnapshot should error');
34+
assert.strictEqual(e.code, 'EACCES');
35+
assert.strictEqual(e.syscall, 'open');
36+
return true;
37+
});
38+
}
39+
2740
[1, true, {}, [], null, Infinity, NaN].forEach((i) => {
2841
assert.throws(() => writeHeapSnapshot(i), {
2942
code: 'ERR_INVALID_ARG_TYPE',

0 commit comments

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