diff --git a/src/main/java/com/intellij/rt/debugger/agent/LogCaptureStorage.java b/src/main/java/com/intellij/rt/debugger/agent/LogCaptureStorage.java index b06bbae..d84981b 100644 --- a/src/main/java/com/intellij/rt/debugger/agent/LogCaptureStorage.java +++ b/src/main/java/com/intellij/rt/debugger/agent/LogCaptureStorage.java @@ -61,8 +61,8 @@ protected Boolean initialValue() { // Raw or packed data can be flushed concurrently, leading to sending the same events multiple times. // It's ok and is handled by the debugger using IDs. static final AtomicLong LAST_FLUSHED_EVENT_ID = new AtomicLong(-1); - static final AtomicLong LAST_PACKED_EVENT_ID = new AtomicLong(-1); - static final AtomicLong LAST_LOGGING_BREAKPOINT_EVENT_ID = new AtomicLong(-1); + private static final AtomicLong LAST_PACKED_EVENT_ID = new AtomicLong(-1); + private static final AtomicLong LAST_LOGGING_BREAKPOINT_EVENT_ID = new AtomicLong(-1); private abstract static class MemoryFootprintEstimate { private final AtomicBoolean removed = new AtomicBoolean(); @@ -139,6 +139,13 @@ public int memoryFootprintEstimate() { private static final int MAX_STACK_DEPTH = 100; // It should be enough, we usually need only a few first frames. + // This state is true if the next line will be the beginning of a line. + // + // Best-effort state. PrintStream synchronizes the common System.out/err path. + // Direct concurrent writes to the same file descriptor don't have a reliable line order for us to preserve. + private static volatile boolean lineStartStateStdout = true; + private static volatile boolean lineStartStateStderr = true; + private static boolean batchingSchedulerStarted; static ArrayList outputWrittenDumpForTests = null; @@ -216,11 +223,14 @@ public static void capture(FileDescriptor fd, byte[] bytes, int off, int len) { if (!STDOUT_CAPTURE_ENABLED) return; boolean isErr = fd == FD_ERR; + OutputSlice outputSlice = findOutputSliceToCapture(isErr, bytes, off, len); + if (outputSlice == null) return; + long id = createNextEventId(Event.STD_OUTPUT_TYPE); ByteArrayOutputStream bas = new ByteArrayOutputStream(); // no need to close it try (DataOutputStream dos = new DataOutputStream(bas)) { - dos.writeInt(len); - dos.write(bytes, off, len); + dos.writeInt(outputSlice.len); + dos.write(bytes, outputSlice.off, outputSlice.len); dos.writeBoolean(isErr); } byte[] payload = bas.toByteArray(); @@ -232,6 +242,58 @@ public static void capture(FileDescriptor fd, byte[] bytes, int off, int len) { } } + private static OutputSlice findOutputSliceToCapture(boolean isErr, byte[] bytes, int off, int len) { + boolean wasAtLineStart = isErr ? lineStartStateStderr : lineStartStateStdout; + assert len > 0; + boolean willBeAtLineStart = isLineSeparator(bytes[off + len - 1]); + if (isErr) { + lineStartStateStderr = willBeAtLineStart; + } else { + lineStartStateStdout = willBeAtLineStart; + } + + if (wasAtLineStart) { + // Regular string at the beginning of the line. + return new OutputSlice(off, len); + } + + // It's some kind of suffix. + // But we match only prefixes or whole lines. So we skip suffix part. + + int lineSep = findFirstLineSeparator(bytes, off, len); + if (lineSep == -1) { + // It's an infix string. We can ignore it completely. + return null; + } + + // It's a suffix string. We should capture it everything starting from the line separator. + // We could skip it if it's just a line break, but we leave it for consistency, to preserve all line breaks. + return new OutputSlice(lineSep, len + off - lineSep); + } + + private static int findFirstLineSeparator(byte[] bytes, int off, int len) { + for (int i = off; i < off + len; i++) { + if (isLineSeparator(bytes[i])) { + return i; + } + } + return -1; + } + + private static boolean isLineSeparator(byte b) { + return b == '\n' || b == '\r'; + } + + private static class OutputSlice { + final int off; + final int len; + + OutputSlice(int off, int len) { + this.off = off; + this.len = len; + } + } + private static boolean hasBatchedLoggingBreakpointEvents() { return LAST_LOGGING_BREAKPOINT_EVENT_ID.get() > LAST_FLUSHED_EVENT_ID.get(); } @@ -473,4 +535,18 @@ private static void setIfGreater(AtomicLong maxValue, long newValue) { } } } + + static void resetStateForTests() { + EVENT_COUNTER.set(0); + LAST_FLUSHED_EVENT_ID.set(-1); + LAST_PACKED_EVENT_ID.set(-1); + LAST_LOGGING_BREAKPOINT_EVENT_ID.set(-1); + EVENTS.clear(); + EVENTS_PAYLOAD_BYTES.set(0); + PACKED_BATCHES.clear(); + PACKED_BATCHES_BYTES.set(0); + outputWrittenDumpForTests = null; + lineStartStateStdout = true; + lineStartStateStderr = true; + } } diff --git a/src/test/java/com/intellij/rt/debugger/agent/LogCaptureEncodingTest.java b/src/test/java/com/intellij/rt/debugger/agent/LogCaptureEncodingTest.java index 4cd5983..0923375 100644 --- a/src/test/java/com/intellij/rt/debugger/agent/LogCaptureEncodingTest.java +++ b/src/test/java/com/intellij/rt/debugger/agent/LogCaptureEncodingTest.java @@ -58,6 +58,87 @@ public void batchesCapturedStdoutEvents() throws Exception { } } + @Test + public void capturesOnlyStdoutPartsStartingAtLineStart() throws Exception { + LogCaptureStorage.init(properties, true); + + capture(FileDescriptor.out, "abc"); + capture(FileDescriptor.out, "def\nxyz"); + + try (DataInputStream is = openPackedBatch(LogCaptureStorage.packBatchedData())) { + assertEquals(2, is.readInt()); + readAndCheckStdoutEvent(0, false, "abc", is); + readAndCheckStdoutEvent(1, false, "\nxyz", is); + } + } + + @Test + public void treatsCarriageReturnAsLineSeparatorForStdoutCapture() throws Exception { + LogCaptureStorage.init(properties, true); + + capture(FileDescriptor.out, "abc"); + capture(FileDescriptor.out, "def\rxyz"); + + try (DataInputStream is = openPackedBatch(LogCaptureStorage.packBatchedData())) { + assertEquals(2, is.readInt()); + readAndCheckStdoutEvent(0, false, "abc", is); + readAndCheckStdoutEvent(1, false, "\rxyz", is); + } + } + + @Test + public void ignoresMidLineStdoutChunkWithoutLineSeparator() throws Exception { + LogCaptureStorage.init(properties, true); + + capture(FileDescriptor.out, "abc"); + capture(FileDescriptor.out, "def"); + capture(FileDescriptor.out, "\nxyz"); + + assertEquals("ignored chunks must not consume event ids", 2, LogCaptureStorage.EVENT_COUNTER.get()); + try (DataInputStream is = openPackedBatch(LogCaptureStorage.packBatchedData())) { + assertEquals(2, is.readInt()); + readAndCheckStdoutEvent(0, false, "abc", is); + readAndCheckStdoutEvent(1, false, "\nxyz", is); + } + } + + @Test + public void capturesMidLineStdoutChunkEndingWithLineSeparator() throws Exception { + LogCaptureStorage.init(properties, true); + + capture(FileDescriptor.out, "abc"); + capture(FileDescriptor.out, "def\n"); + capture(FileDescriptor.out, "xyz"); + + try (DataInputStream is = openPackedBatch(LogCaptureStorage.packBatchedData())) { + assertEquals(3, is.readInt()); + readAndCheckStdoutEvent(0, false, "abc", is); + readAndCheckStdoutEvent(1, false, "\n", is); + readAndCheckStdoutEvent(2, false, "xyz", is); + } + } + + @Test + public void keepsStdoutAndStderrLineStartStatesSeparate() throws Exception { + LogCaptureStorage.init(properties, true); + + capture(FileDescriptor.out, "stdout"); + capture(FileDescriptor.err, "stderr"); + capture(FileDescriptor.out, "ignored stdout"); + capture(FileDescriptor.err, "ignored stderr"); + capture(FileDescriptor.out, "\nnext stdout"); + capture(FileDescriptor.err, "\rnext stderr"); + + assertEquals("ignored chunks must not consume event ids", 4, LogCaptureStorage.EVENT_COUNTER.get()); + try (DataInputStream is = openPackedBatch(LogCaptureStorage.packBatchedData())) { + assertEquals(4, is.readInt()); + readAndCheckStdoutEvent(0, false, "stdout", is); + readAndCheckStdoutEvent(1, true, "stderr", is); + readAndCheckStdoutEvent(2, false, "\nnext stdout", is); + readAndCheckStdoutEvent(3, true, "\rnext stderr", is); + } + } + @Test public void batchesLoggingBreakpointEvents() throws Exception { LogCaptureStorage.init(properties, false); @@ -552,15 +633,7 @@ private static Throwable throwableWithStackDepth(int depth) { } static void resetLogCaptureStorage() { - LogCaptureStorage.EVENT_COUNTER.set(0); - LogCaptureStorage.LAST_FLUSHED_EVENT_ID.set(-1); - LogCaptureStorage.LAST_PACKED_EVENT_ID.set(-1); - LogCaptureStorage.LAST_LOGGING_BREAKPOINT_EVENT_ID.set(-1); - LogCaptureStorage.EVENTS.clear(); - LogCaptureStorage.EVENTS_PAYLOAD_BYTES.set(0); - LogCaptureStorage.PACKED_BATCHES.clear(); - LogCaptureStorage.PACKED_BATCHES_BYTES.set(0); - LogCaptureStorage.outputWrittenDumpForTests = null; + LogCaptureStorage.resetStateForTests(); ThrowableInterner.clear(); STACK_DICTIONARY.remove(); } diff --git a/src/test/java/com/intellij/rt/debugger/agent/ThrowableCapacityOverhead.java b/src/test/java/com/intellij/rt/debugger/agent/ThrowableCapacityOverhead.java index f8b6198..b17a7de 100644 --- a/src/test/java/com/intellij/rt/debugger/agent/ThrowableCapacityOverhead.java +++ b/src/test/java/com/intellij/rt/debugger/agent/ThrowableCapacityOverhead.java @@ -157,14 +157,7 @@ private static void measureAfterBatchZipping(int stackDepth, int repeats) throws } private static void resetLogCaptureStorage() { - LogCaptureStorage.EVENT_COUNTER.set(0); - LogCaptureStorage.LAST_FLUSHED_EVENT_ID.set(-1); - LogCaptureStorage.LAST_LOGGING_BREAKPOINT_EVENT_ID.set(-1); - LogCaptureStorage.EVENTS.clear(); - LogCaptureStorage.EVENTS_PAYLOAD_BYTES.set(0); - LogCaptureStorage.PACKED_BATCHES.clear(); - LogCaptureStorage.PACKED_BATCHES_BYTES.set(0); - LogCaptureStorage.outputWrittenDumpForTests = null; + LogCaptureStorage.resetStateForTests(); ThrowableInterner.clear(); }