From 30ba0fc2e9bad50c3ed787baa98e063d5c2f9344 Mon Sep 17 00:00:00 2001 From: Leonid Shalupov Date: Wed, 31 Jan 2018 19:07:03 +0100 Subject: [PATCH] Store big exception attachments on disk, this prevents OOM in Rider on attaching compressed logs IDEA-CR-32281 --- .../diagnostic/AttachmentFactory.java | 60 ++++++++---- .../diagnostic/AttachmentFactoryTest.java | 42 ++++++++ .../openapi/diagnostic/Attachment.java | 97 ++++++++++++++++--- 3 files changed, 170 insertions(+), 29 deletions(-) create mode 100644 platform/platform-tests/testSrc/com/intellij/diagnostic/AttachmentFactoryTest.java diff --git a/platform/platform-impl/src/com/intellij/diagnostic/AttachmentFactory.java b/platform/platform-impl/src/com/intellij/diagnostic/AttachmentFactory.java index 9e107edcc140..d19d59a991b1 100644 --- a/platform/platform-impl/src/com/intellij/diagnostic/AttachmentFactory.java +++ b/platform/platform-impl/src/com/intellij/diagnostic/AttachmentFactory.java @@ -1,22 +1,27 @@ package com.intellij.diagnostic; import com.intellij.openapi.diagnostic.Attachment; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; import com.intellij.openapi.fileEditor.FileDocumentManager; -import com.intellij.openapi.fileEditor.impl.LoadTextUtil; import com.intellij.openapi.util.io.FileUtil; +import com.intellij.openapi.vfs.CharsetToolkit; import com.intellij.openapi.vfs.VirtualFile; import org.jetbrains.annotations.NotNull; -import java.io.File; -import java.io.IOException; +import java.io.*; import java.text.MessageFormat; /** * @author yole */ public class AttachmentFactory { + private static final Logger LOG = Logger.getInstance(AttachmentFactory.class); + + private static final Long BIG_FILE_THRESHOLD_BYTES = 50 * 1024L; + private static final String ERROR_MESSAGE_PATTERN = "[[[Can't get file contents: {0}]]]"; + private static final String BIG_FILE_MESSAGE_PATTERN = "[[[File is too big to display: {0}]]]"; public static Attachment createAttachment(Document document) { VirtualFile file = FileDocumentManager.getInstance().getFile(document); @@ -24,30 +29,51 @@ public class AttachmentFactory { } public static Attachment createAttachment(@NotNull VirtualFile file) { - return new Attachment(file.getPresentableUrl(), getBytes(file), - file.getFileType().isBinary() ? "File is binary" : LoadTextUtil.loadText(file).toString()); + try { + boolean isBinary = file.getFileType().isBinary(); + boolean isBigFile = file.getLength() > BIG_FILE_THRESHOLD_BYTES; + + try (InputStream inputStream = file.getInputStream()) { + return createAttachment(file.getPresentableUrl(), inputStream, isBinary, isBigFile); + } + } catch (IOException e) { + return handleException(e, file.getName(), file.getPath()); + } } public static Attachment createAttachment(@NotNull File file, boolean isBinary) { - byte[] bytes = getBytes(file); - return new Attachment(file.getPath(), bytes, isBinary ? "File is binary" : new String(bytes)); - } - - private static byte[] getBytes(File file) { try { - return FileUtil.loadFileBytes(file); - } - catch (IOException e) { - return Attachment.getBytes(MessageFormat.format(ERROR_MESSAGE_PATTERN, e.getMessage())); + try (InputStream inputStream = new FileInputStream(file)) { + return createAttachment(file.getPath(), inputStream, isBinary, file.length() > BIG_FILE_THRESHOLD_BYTES); + } + } catch (IOException e) { + return handleException(e, file.getName(), file.getPath()); } } - private static byte[] getBytes(VirtualFile file) { + private static Attachment handleException(Throwable t, String name, String moniker) { + final String errorMessage = MessageFormat.format(ERROR_MESSAGE_PATTERN, t.getMessage()); + + LOG.warn("Unable to create Attachment from " + moniker + ": " + t.getMessage(), t); + return new Attachment(name, errorMessage); + } + + private static Attachment createAttachment(@NotNull String path, InputStream contentStream, boolean isBinary, boolean isBigFile) { + if (isBigFile) { + return new Attachment(path, contentStream, MessageFormat.format(BIG_FILE_MESSAGE_PATTERN, path)); + } else { + byte[] bytes = getBytes(contentStream); + final String displayText = isBinary ? "[File is binary]" : new String(bytes, CharsetToolkit.UTF8_CHARSET); + return new Attachment(path, bytes, displayText); + } + } + + private static byte[] getBytes(InputStream inputStream) { try { - return file.contentsToByteArray(); + return FileUtil.loadBytes(inputStream); } catch (IOException e) { - return Attachment.getBytes(MessageFormat.format(ERROR_MESSAGE_PATTERN, e.getMessage())); + return MessageFormat.format(ERROR_MESSAGE_PATTERN, e.getMessage()).getBytes(CharsetToolkit.UTF8_CHARSET); } } } diff --git a/platform/platform-tests/testSrc/com/intellij/diagnostic/AttachmentFactoryTest.java b/platform/platform-tests/testSrc/com/intellij/diagnostic/AttachmentFactoryTest.java new file mode 100644 index 000000000000..39d22b4ddec7 --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/diagnostic/AttachmentFactoryTest.java @@ -0,0 +1,42 @@ +package com.intellij.diagnostic; + +import com.intellij.openapi.diagnostic.Attachment; +import com.intellij.openapi.util.io.FileUtil; +import org.junit.Assert; +import org.junit.Test; + +import java.io.*; + +public class AttachmentFactoryTest { + @Test + public void testBigFilesStoredOnDisk() throws IOException { + final File testFile = FileUtil.createTempFile("test", ".bin", true); + try { + FileUtil.writeToFile(testFile, new byte[150000]); + Attachment attachment = AttachmentFactory.createAttachment(testFile, true); + + try (InputStream contentStream = attachment.openContentStream()) { + Assert.assertTrue(contentStream instanceof FileInputStream); + } + } finally { + //noinspection ResultOfMethodCallIgnored + testFile.delete(); + } + } + + @Test + public void testSmallFilesStoredInMemory() throws IOException { + final File testFile = FileUtil.createTempFile("test", ".bin", true); + try { + FileUtil.writeToFile(testFile, new byte[1500]); + Attachment attachment = AttachmentFactory.createAttachment(testFile, true); + + try (InputStream contentStream = attachment.openContentStream()) { + Assert.assertTrue(contentStream instanceof ByteArrayInputStream); + } + } finally { + //noinspection ResultOfMethodCallIgnored + testFile.delete(); + } + } +} diff --git a/platform/util/src/com/intellij/openapi/diagnostic/Attachment.java b/platform/util/src/com/intellij/openapi/diagnostic/Attachment.java index c1ba67787b37..43de33e03372 100644 --- a/platform/util/src/com/intellij/openapi/diagnostic/Attachment.java +++ b/platform/util/src/com/intellij/openapi/diagnostic/Attachment.java @@ -15,29 +15,76 @@ */ package com.intellij.openapi.diagnostic; +import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.CharsetToolkit; +import com.intellij.util.ArrayUtil; import com.intellij.util.Base64; import com.intellij.util.ExceptionUtil; import com.intellij.util.PathUtilRt; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.*; public class Attachment { + private static final Logger LOG = Logger.getInstance(Attachment.class); + public static final Attachment[] EMPTY_ARRAY = new Attachment[0]; + private final String myPath; - private final byte[] myBytes; + @Nullable private final File myTemporaryFile; + @Nullable private final byte[] myBytes; private boolean myIncluded; // opt-out for traces, opt-in otherwise private final String myDisplayText; public Attachment(@NotNull String path, @NotNull String content) { - myPath = path; - myDisplayText = content; - myBytes = getBytes(content); + this(path, content.getBytes(CharsetToolkit.UTF8_CHARSET), content); } public Attachment(@NotNull String path, @NotNull byte[] bytes, @NotNull String displayText) { myPath = path; - myBytes = bytes; myDisplayText = displayText; + myBytes = bytes; + myTemporaryFile = null; + } + + public Attachment(@NotNull String path, @NotNull InputStream inputStream, @NotNull String displayText) { + myPath = path; + myDisplayText = displayText; + + myBytes = null; + + File temporaryFile; + try { + temporaryFile = FileUtil.createTempFile("intellij-attachment", ".bin", true); + temporaryFile.deleteOnExit(); + } catch (IOException e) { + LOG.error("Unable to create temp file for attachment: " + e.getMessage(), e); + temporaryFile = null; + } + + if (temporaryFile != null) { + try { + OutputStream outputStream = new FileOutputStream(temporaryFile); + try { + FileUtil.copy(inputStream, outputStream); + } finally { + outputStream.close(); + } + } catch (IOException e) { + LOG.error("Unable to write temp file for attachment at " + temporaryFile + ": " + e.getMessage(), e); + temporaryFile = null; + } + } + + myTemporaryFile = temporaryFile; + } + + public Attachment(@NotNull String path, @NotNull File existingTemporaryFile, @NotNull String displayText) { + myPath = path; + myDisplayText = displayText; + myTemporaryFile = existingTemporaryFile; + myBytes = null; } public Attachment(@NotNull String name, @NotNull Throwable throwable) { @@ -45,11 +92,6 @@ public class Attachment { myIncluded = true; } - @NotNull - public static byte[] getBytes(@NotNull String content) { - return content.getBytes(CharsetToolkit.UTF8_CHARSET); - } - @NotNull public String getDisplayText() { return myDisplayText; @@ -67,12 +109,43 @@ public class Attachment { @NotNull public String getEncodedBytes() { - return Base64.encode(myBytes); + return Base64.encode(getBytes()); } @NotNull public byte[] getBytes() { - return myBytes; + if (myBytes != null) { + return myBytes; + } + + if (myTemporaryFile == null) { + return ArrayUtil.EMPTY_BYTE_ARRAY; + } + + try { + return FileUtil.loadFileBytes(myTemporaryFile); + } catch (IOException e) { + LOG.error("Unable to read attachment content from temporary file " + myTemporaryFile + ": " + e.getMessage(), e); + return ArrayUtil.EMPTY_BYTE_ARRAY; + } + } + + @NotNull + public InputStream openContentStream() { + if (myBytes != null) { + return new ByteArrayInputStream(myBytes); + } + + if (myTemporaryFile == null) { + return new ByteArrayInputStream(ArrayUtil.EMPTY_BYTE_ARRAY); + } + + try { + return new FileInputStream(myTemporaryFile); + } catch (FileNotFoundException e) { + LOG.warn("Unable to read attachment content from temporary file " + myTemporaryFile + ": " + e.getMessage(), e); + return new ByteArrayInputStream(ArrayUtil.EMPTY_BYTE_ARRAY); + } } public boolean isIncluded() {