Allow heap dump requests with a FileDescriptor arg.
Part of a larger change to allow "am" to initiate hprof heap dumps.
The original implementation wrote data to a temp file, because we
compute the second part first while doing GC work. Now all paths
work like DDMS, writing all data to a memstream and then copying it
out to a file at the very end.
Also, fix a potential fd leak on failure in the profiling code.
Bug 2759474.
Change-Id: I0f838cbd9948a23088f08463c3008be7198d76e7
diff --git a/vm/hprof/Hprof.c b/vm/hprof/Hprof.c
index 0b95aad..bcad8b1 100644
--- a/vm/hprof/Hprof.c
+++ b/vm/hprof/Hprof.c
@@ -25,6 +25,7 @@
#include <string.h>
#include <unistd.h>
+#include <fcntl.h>
#include <errno.h>
#include <sys/time.h>
#include <time.h>
@@ -33,35 +34,8 @@
#define kHeadSuffix "-hptemp"
hprof_context_t *
-hprofStartup(const char *outputFileName, bool directToDdms)
+hprofStartup(const char *outputFileName, int fd, bool directToDdms)
{
- FILE* fp = NULL;
-
- if (!directToDdms) {
- int len = strlen(outputFileName);
- char fileName[len + sizeof(kHeadSuffix)];
-
- /* Construct the temp file name. This wasn't handed to us by the
- * application, so we need to be careful about stomping on it.
- */
- sprintf(fileName, "%s" kHeadSuffix, outputFileName);
- if (access(fileName, F_OK) == 0) {
- LOGE("hprof: temp file %s exists, bailing\n", fileName);
- return NULL;
- }
-
- fp = fopen(fileName, "w+");
- if (fp == NULL) {
- LOGE("hprof: can't open %s: %s.\n", fileName, strerror(errno));
- return NULL;
- }
- if (unlink(fileName) != 0) {
- LOGW("hprof: WARNING: unable to remove temp file %s\n", fileName);
- /* keep going */
- }
- LOGI("hprof: dumping VM heap to \"%s\".\n", fileName);
- }
-
hprofStartup_String();
hprofStartup_Class();
#if WITH_HPROF_STACK
@@ -72,70 +46,26 @@
hprof_context_t *ctx = malloc(sizeof(*ctx));
if (ctx == NULL) {
LOGE("hprof: can't allocate context.\n");
- if (fp != NULL)
- fclose(fp);
return NULL;
}
- /* pass in "fp" for the temp file, and the name of the output file */
- hprofContextInit(ctx, strdup(outputFileName), fp, false, directToDdms);
+ /* pass in name or descriptor of the output file */
+ hprofContextInit(ctx, strdup(outputFileName), fd, false, directToDdms);
- assert(ctx->fp != NULL);
+ assert(ctx->memFp != NULL);
return ctx;
}
/*
- * Copy the entire contents of "srcFp" to "dstFp".
- *
- * Returns "true" on success.
- */
-static bool
-copyFileToFile(FILE *dstFp, FILE *srcFp)
-{
- char buf[65536];
- size_t dataRead, dataWritten;
-
- while (true) {
- dataRead = fread(buf, 1, sizeof(buf), srcFp);
- if (dataRead > 0) {
- dataWritten = fwrite(buf, 1, dataRead, dstFp);
- if (dataWritten != dataRead) {
- LOGE("hprof: failed writing data (%d of %d): %s\n",
- dataWritten, dataRead, strerror(errno));
- return false;
- }
- } else {
- if (feof(srcFp))
- return true;
- LOGE("hprof: failed reading data (res=%d): %s\n",
- dataRead, strerror(errno));
- return false;
- }
- }
-}
-
-/*
* Finish up the hprof dump. Returns true on success.
*/
bool
hprofShutdown(hprof_context_t *tailCtx)
{
- FILE *fp = NULL;
-
- /* flush output to the temp file, then prepare the output file */
+ /* flush the "tail" portion of the output */
hprofFlushCurrentRecord(tailCtx);
- LOGI("hprof: dumping heap strings to \"%s\".\n", tailCtx->fileName);
- if (!tailCtx->directToDdms) {
- fp = fopen(tailCtx->fileName, "w");
- if (fp == NULL) {
- LOGE("can't open %s: %s\n", tailCtx->fileName, strerror(errno));
- hprofFreeContext(tailCtx);
- return false;
- }
- }
-
/*
* Create a new context struct for the start of the file. We
* heap-allocate it so we can share the "free" function.
@@ -143,14 +73,13 @@
hprof_context_t *headCtx = malloc(sizeof(*headCtx));
if (headCtx == NULL) {
LOGE("hprof: can't allocate context.\n");
- if (fp != NULL)
- fclose(fp);
hprofFreeContext(tailCtx);
- return NULL;
+ return false;
}
- hprofContextInit(headCtx, strdup(tailCtx->fileName), fp, true,
+ hprofContextInit(headCtx, strdup(tailCtx->fileName), tailCtx->fd, true,
tailCtx->directToDdms);
+ LOGI("hprof: dumping heap strings to \"%s\".\n", tailCtx->fileName);
hprofDumpStrings(headCtx);
hprofDumpClasses(headCtx);
@@ -176,11 +105,11 @@
hprofShutdown_StackFrame();
#endif
- if (tailCtx->directToDdms) {
- /* flush to ensure memstream pointer and size are updated */
- fflush(headCtx->fp);
- fflush(tailCtx->fp);
+ /* flush to ensure memstream pointer and size are updated */
+ fflush(headCtx->memFp);
+ fflush(tailCtx->memFp);
+ if (tailCtx->directToDdms) {
/* send the data off to DDMS */
struct iovec iov[2];
iov[0].iov_base = headCtx->fileDataPtr;
@@ -190,22 +119,50 @@
dvmDbgDdmSendChunkV(CHUNK_TYPE("HPDS"), iov, 2);
} else {
/*
- * Append the contents of the temp file to the output file. The temp
- * file was removed immediately after being opened, so it will vanish
- * when we close it.
+ * Open the output file, and copy the head and tail to it.
*/
- rewind(tailCtx->fp);
- if (!copyFileToFile(headCtx->fp, tailCtx->fp)) {
- LOGW("hprof: file copy failed, hprof data may be incomplete\n");
- /* finish up anyway */
+ assert(headCtx->fd == tailCtx->fd);
+
+ int outFd;
+ if (headCtx->fd >= 0) {
+ outFd = dup(headCtx->fd);
+ if (outFd < 0) {
+ LOGE("dup(%d) failed: %s\n", headCtx->fd, strerror(errno));
+ /* continue to fail-handler below */
+ }
+ } else {
+ outFd = open(tailCtx->fileName, O_WRONLY|O_CREAT, 0644);
+ if (outFd < 0) {
+ LOGE("can't open %s: %s\n", headCtx->fileName, strerror(errno));
+ /* continue to fail-handler below */
+ }
+ }
+ if (outFd < 0) {
+ hprofFreeContext(headCtx);
+ hprofFreeContext(tailCtx);
+ return false;
+ }
+
+ int result;
+ result = sysWriteFully(outFd, headCtx->fileDataPtr,
+ headCtx->fileDataSize, "hprof-head");
+ result |= sysWriteFully(outFd, tailCtx->fileDataPtr,
+ tailCtx->fileDataSize, "hprof-tail");
+ close(outFd);
+ if (result != 0) {
+ hprofFreeContext(headCtx);
+ hprofFreeContext(tailCtx);
+ return false;
}
}
+ /* throw out a log message for the benefit of "runhat" */
+ LOGI("hprof: heap dump completed (%dKB)\n",
+ (headCtx->fileDataSize + tailCtx->fileDataSize + 1023) / 1024);
+
hprofFreeContext(headCtx);
hprofFreeContext(tailCtx);
- /* throw out a log message for the benefit of "runhat" */
- LOGI("hprof: heap dump completed, temp file removed\n");
return true;
}
@@ -217,8 +174,10 @@
{
assert(ctx != NULL);
- if (ctx->fp != NULL)
- fclose(ctx->fp);
+ /* we don't own ctx->fd, do not close */
+
+ if (ctx->memFp != NULL)
+ fclose(ctx->memFp);
free(ctx->curRec.body);
free(ctx->fileName);
free(ctx->fileDataPtr);
diff --git a/vm/hprof/Hprof.h b/vm/hprof/Hprof.h
index fd89d06..18f4102 100644
--- a/vm/hprof/Hprof.h
+++ b/vm/hprof/Hprof.h
@@ -133,15 +133,16 @@
size_t objectsInSegment;
/*
- * If "directToDdms" is not set, "fileName" is valid, and "fileDataPtr"
- * and "fileDataSize" are not used. If "directToDdms" is not set,
- * it's the other way around.
+ * If directToDdms is set, "fileName" and "fd" will be ignored.
+ * Otherwise, "fileName" must be valid, though if "fd" >= 0 it will
+ * only be used for debug messages.
*/
bool directToDdms;
char *fileName;
char *fileDataPtr; // for open_memstream
size_t fileDataSize; // for open_memstream
- FILE *fp;
+ FILE *memFp;
+ int fd;
} hprof_context_t;
@@ -187,7 +188,7 @@
* HprofOutput.c functions
*/
-void hprofContextInit(hprof_context_t *ctx, char *fileName, FILE *fp,
+void hprofContextInit(hprof_context_t *ctx, char *fileName, int fd,
bool writeHeader, bool directToDdms);
int hprofFlushRecord(hprof_record_t *rec, FILE *fp);
@@ -244,7 +245,8 @@
* Hprof.c functions
*/
-hprof_context_t* hprofStartup(const char *outputFileName, bool directToDdms);
+hprof_context_t* hprofStartup(const char *outputFileName, int fd,
+ bool directToDdms);
bool hprofShutdown(hprof_context_t *ctx);
void hprofFreeContext(hprof_context_t *ctx);
@@ -255,7 +257,7 @@
* the heap implementation; these functions require heap knowledge,
* so they are implemented in Heap.c.
*/
-int hprofDumpHeap(const char* fileName, bool directToDdms);
+int hprofDumpHeap(const char* fileName, int fd, bool directToDdms);
void dvmHeapSetHprofGcScanState(hprof_heap_tag_t state, u4 threadSerialNumber);
#endif // _DALVIK_HPROF_HPROF
diff --git a/vm/hprof/HprofOutput.c b/vm/hprof/HprofOutput.c
index 0677c85..25512b2 100644
--- a/vm/hprof/HprofOutput.c
+++ b/vm/hprof/HprofOutput.c
@@ -59,32 +59,30 @@
/*
* Initialize an hprof context struct.
*
- * This will take ownership of "fileName" and "fp".
+ * This will take ownership of "fileName".
*/
void
-hprofContextInit(hprof_context_t *ctx, char *fileName, FILE *fp,
+hprofContextInit(hprof_context_t *ctx, char *fileName, int fd,
bool writeHeader, bool directToDdms)
{
memset(ctx, 0, sizeof (*ctx));
- if (directToDdms) {
- /*
- * Have to do this here, because it must happen after we
- * memset the struct (want to treat fileDataPtr/fileDataSize
- * as read-only while the file is open).
- */
- assert(fp == NULL);
- fp = open_memstream(&ctx->fileDataPtr, &ctx->fileDataSize);
- if (fp == NULL) {
- /* not expected */
- LOGE("hprof: open_memstream failed: %s\n", strerror(errno));
- dvmAbort();
- }
+ /*
+ * Have to do this here, because it must happen after we
+ * memset the struct (want to treat fileDataPtr/fileDataSize
+ * as read-only while the file is open).
+ */
+ FILE* fp = open_memstream(&ctx->fileDataPtr, &ctx->fileDataSize);
+ if (fp == NULL) {
+ /* not expected */
+ LOGE("hprof: open_memstream failed: %s\n", strerror(errno));
+ dvmAbort();
}
ctx->directToDdms = directToDdms;
ctx->fileName = fileName;
- ctx->fp = fp;
+ ctx->memFp = fp;
+ ctx->fd = fd;
ctx->curRec.allocLen = 128;
ctx->curRec.body = malloc(ctx->curRec.allocLen);
@@ -158,7 +156,7 @@
int
hprofFlushCurrentRecord(hprof_context_t *ctx)
{
- return hprofFlushRecord(&ctx->curRec, ctx->fp);
+ return hprofFlushRecord(&ctx->curRec, ctx->memFp);
}
int
@@ -167,7 +165,7 @@
hprof_record_t *rec = &ctx->curRec;
int err;
- err = hprofFlushRecord(rec, ctx->fp);
+ err = hprofFlushRecord(rec, ctx->memFp);
if (err != 0) {
return err;
} else if (rec->dirty) {