mirror of
https://github.com/rubyjs/mini_racer
synced 2023-03-27 23:21:28 -04:00
rb_heap_snapshot: fix memory leak from fdopen
The result of fdopen(3) must be matched with fclose(3) to avoid memory leaks. However, fclose(3) has the side-effect of closing the underlying FD behind Ruby's back (which would corrupt Ruby's internals and cause problems for the remainder of the process lifetime). While one could dup(fptr->fd) to fdopen+fclose the resulting fd, it would cause problems on FD-constrained systems. Since GetChunkSize() is available and returns a reasonable value (64K), the buffering of fwrite(3) is unnecessary and we can safely use write(2) without performance loss. So use unbuffered write(2) and perhaps prepare us for better error reporting in a future change. I've verified WriteAsciiChunk is indeed receiving the requested 64K (or close to it) in nearly all cases, so excessive syscalls should not be a problem.
This commit is contained in:
parent
2725152c9d
commit
0752264620
1 changed files with 26 additions and 14 deletions
|
@ -11,6 +11,7 @@
|
||||||
#include <mutex>
|
#include <mutex>
|
||||||
#include <atomic>
|
#include <atomic>
|
||||||
#include <math.h>
|
#include <math.h>
|
||||||
|
#include <errno.h>
|
||||||
|
|
||||||
using namespace v8;
|
using namespace v8;
|
||||||
|
|
||||||
|
@ -1599,7 +1600,9 @@ rb_heap_stats(VALUE self) {
|
||||||
// https://github.com/bnoordhuis/node-heapdump/blob/master/src/heapdump.cc
|
// https://github.com/bnoordhuis/node-heapdump/blob/master/src/heapdump.cc
|
||||||
class FileOutputStream : public OutputStream {
|
class FileOutputStream : public OutputStream {
|
||||||
public:
|
public:
|
||||||
FileOutputStream(FILE* stream) : stream_(stream) {}
|
int err;
|
||||||
|
|
||||||
|
FileOutputStream(int fd) : fd(fd) { err = 0; }
|
||||||
|
|
||||||
virtual int GetChunkSize() {
|
virtual int GetChunkSize() {
|
||||||
return 65536;
|
return 65536;
|
||||||
|
@ -1608,17 +1611,27 @@ class FileOutputStream : public OutputStream {
|
||||||
virtual void EndOfStream() {}
|
virtual void EndOfStream() {}
|
||||||
|
|
||||||
virtual WriteResult WriteAsciiChunk(char* data, int size) {
|
virtual WriteResult WriteAsciiChunk(char* data, int size) {
|
||||||
const size_t len = static_cast<size_t>(size);
|
size_t len = static_cast<size_t>(size);
|
||||||
size_t off = 0;
|
|
||||||
|
|
||||||
while (off < len && !feof(stream_) && !ferror(stream_))
|
while (len) {
|
||||||
off += fwrite(data + off, 1, len - off, stream_);
|
ssize_t w = write(fd, data, len);
|
||||||
|
|
||||||
return off == len ? kContinue : kAbort;
|
if (w > 0) {
|
||||||
|
data += w;
|
||||||
|
len -= w;
|
||||||
|
} else if (w < 0) {
|
||||||
|
err = errno;
|
||||||
|
return kAbort;
|
||||||
|
} else { /* w == 0, could be out-of-space */
|
||||||
|
err = -1;
|
||||||
|
return kAbort;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return kContinue;
|
||||||
}
|
}
|
||||||
|
|
||||||
private:
|
private:
|
||||||
FILE* stream_;
|
int fd;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
||||||
|
@ -1631,10 +1644,8 @@ rb_heap_snapshot(VALUE self, VALUE file) {
|
||||||
|
|
||||||
if (!fptr) return Qfalse;
|
if (!fptr) return Qfalse;
|
||||||
|
|
||||||
FILE* fp;
|
// prepare for unbuffered write(2) below:
|
||||||
fp = fdopen(fptr->fd, "w");
|
rb_funcall(file, rb_intern("flush"), 0);
|
||||||
if (fp == NULL) return Qfalse;
|
|
||||||
|
|
||||||
|
|
||||||
ContextInfo* context_info;
|
ContextInfo* context_info;
|
||||||
TypedData_Get_Struct(self, ContextInfo, &context_type, context_info);
|
TypedData_Get_Struct(self, ContextInfo, &context_type, context_info);
|
||||||
|
@ -1651,13 +1662,14 @@ rb_heap_snapshot(VALUE self, VALUE file) {
|
||||||
|
|
||||||
const HeapSnapshot* const snap = heap_profiler->TakeHeapSnapshot();
|
const HeapSnapshot* const snap = heap_profiler->TakeHeapSnapshot();
|
||||||
|
|
||||||
FileOutputStream stream(fp);
|
FileOutputStream stream(fptr->fd);
|
||||||
snap->Serialize(&stream, HeapSnapshot::kJSON);
|
snap->Serialize(&stream, HeapSnapshot::kJSON);
|
||||||
|
|
||||||
fflush(fp);
|
|
||||||
|
|
||||||
const_cast<HeapSnapshot*>(snap)->Delete();
|
const_cast<HeapSnapshot*>(snap)->Delete();
|
||||||
|
|
||||||
|
/* TODO: perhaps rb_sys_fail here */
|
||||||
|
if (stream.err) return Qfalse;
|
||||||
|
|
||||||
return Qtrue;
|
return Qtrue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue