Skip to content

Commit 5d64cca

Browse files
authored
Improve tests
1 parent fe47670 commit 5d64cca

File tree

2 files changed

+85
-88
lines changed

2 files changed

+85
-88
lines changed

log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/RootThrowablePatternConverterTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.logging.log4j.core.pattern;
1818

1919
import static java.util.Arrays.asList;
20+
import static org.apache.logging.log4j.core.pattern.ThrowablePatternConverterTest.EXCEPTION;
2021
import static org.apache.logging.log4j.core.pattern.ThrowablePatternConverterTest.LEVEL;
2122
import static org.apache.logging.log4j.core.pattern.ThrowablePatternConverterTest.convert;
2223
import static org.assertj.core.api.Assertions.assertThat;
@@ -37,8 +38,6 @@
3738
*/
3839
class RootThrowablePatternConverterTest {
3940

40-
static final Throwable EXCEPTION = TestFriendlyException.INSTANCE;
41-
4241
private static final StackTraceElement THROWING_METHOD =
4342
Throwables.getRootCause(EXCEPTION).getStackTrace()[0];
4443

log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java

Lines changed: 84 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
import java.io.PrintStream;
2626
import java.nio.charset.Charset;
2727
import java.nio.charset.StandardCharsets;
28+
import java.util.Arrays;
2829
import java.util.List;
2930
import java.util.concurrent.CountDownLatch;
3031
import java.util.concurrent.ExecutorService;
3132
import java.util.concurrent.Executors;
32-
import java.util.concurrent.TimeUnit;
3333
import java.util.concurrent.atomic.AtomicInteger;
3434
import java.util.concurrent.locks.LockSupport;
35+
import java.util.function.Consumer;
36+
import java.util.function.Function;
3537
import java.util.stream.Collectors;
3638
import java.util.stream.IntStream;
3739
import java.util.stream.Stream;
@@ -444,105 +446,101 @@ private static String normalizeStackTrace(final String stackTrace, final String
444446
.replaceAll(" ~\\[\\?:[^]]+](\\Q" + conversionEnding + "\\E|$)", " ~[?:0]$1");
445447
}
446448

447-
@Test
449+
@RepeatedTest(10)
448450
@Issue("https://github.com/apache/logging-log4j2/issues/3940")
449-
void concurrent_stacktrace_mutation_should_not_cause_failure() throws Exception {
450-
final ExecutorService executor = Executors.newFixedThreadPool(2);
451+
void concurrent_stack_trace_mutation_should_not_cause_failure() throws Exception {
452+
final int stackTracePerThreadCount = 100;
453+
formatThrowableWhileMutatingConcurrently(threadIndex -> {
454+
final List<StackTraceElement[]> stackTraces = createExceptionsOfDifferentDepths().stream()
455+
.map(Throwable::getStackTrace)
456+
.collect(Collectors.toList());
457+
return exception -> {
458+
for (int stackTraceIndex = 0; stackTraceIndex < stackTracePerThreadCount; stackTraceIndex++) {
459+
exception.setStackTrace(stackTraces.get(stackTraceIndex));
460+
// Give some time slack to increase randomness
461+
LockSupport.parkNanos(1);
462+
}
463+
};
464+
});
465+
}
451466

452-
// Create the formatter
453-
final List<PatternFormatter> patternFormatters = PATTERN_PARSER.parse(patternPrefix, false, true, true);
454-
assertThat(patternFormatters).hasSize(1);
455-
final PatternFormatter patternFormatter = patternFormatters.get(0);
456-
final StringBuilder buffer = new StringBuilder();
467+
@RepeatedTest(10)
468+
@Issue("https://github.com/apache/logging-log4j2/issues/3929")
469+
void concurrent_suppressed_mutation_should_not_cause_failure() throws Exception {
470+
formatThrowableWhileMutatingConcurrently(threadIndex -> {
471+
final List<Exception> exceptions = createExceptionsOfDifferentDepths();
472+
return exception -> exceptions.forEach(suppressed -> {
473+
exception.addSuppressed(suppressed);
474+
// Give some time slack to increase randomness
475+
LockSupport.parkNanos(1);
476+
});
477+
});
478+
}
457479

458-
try {
459-
for (int i = 0; i < 10; i++) {
460-
final CountDownLatch latch = new CountDownLatch(2);
461-
final Throwable exception = new RuntimeException();
462-
final LogEvent logEvent = Log4jLogEvent.newBuilder()
463-
.setThrown(exception)
464-
.setLevel(LEVEL)
465-
.build();
480+
private void formatThrowableWhileMutatingConcurrently(
481+
Function<Integer, Consumer<Throwable>> throwableMutatorFactory) throws Exception {
466482

483+
// Test constants
484+
final int threadCount = Math.max(8, Runtime.getRuntime().availableProcessors());
485+
final ExecutorService executor = Executors.newFixedThreadPool(threadCount);
486+
try {
487+
final Exception exception = new Exception();
488+
final CountDownLatch startLatch =
489+
new CountDownLatch(threadCount + /* the main thread invoking the rendering: */ 1);
490+
final AtomicInteger runningThreadCountRef = new AtomicInteger(threadCount);
491+
492+
// Schedule threads that will start mutating the exception with the start signal
493+
for (int threadIndex = 0; threadIndex < threadCount; threadIndex++) {
494+
final Consumer<Throwable> exceptionMutator = throwableMutatorFactory.apply(threadIndex);
467495
executor.submit(() -> {
468496
try {
469-
patternFormatter.format(logEvent, buffer);
470-
buffer.setLength(0);
471-
latch.countDown();
472-
} catch (Throwable e) {
473-
e.printStackTrace();
497+
startLatch.countDown();
498+
startLatch.await();
499+
exceptionMutator.accept(exception);
500+
} catch (InterruptedException ignored) {
501+
// Restore the interrupt
502+
Thread.currentThread().interrupt();
503+
} finally {
504+
runningThreadCountRef.decrementAndGet();
474505
}
475506
});
476-
executor.submit(() -> {
477-
exception.setStackTrace(new StackTraceElement[0]);
478-
latch.countDown();
479-
});
480-
if (latch.await(1, TimeUnit.SECONDS) == false) {
481-
throw new IllegalStateException("timed out waiting for tasks to complete");
482-
}
507+
}
508+
509+
// Create the formatter
510+
final List<PatternFormatter> patternFormatters = PATTERN_PARSER.parse(patternPrefix, false, true, true);
511+
assertThat(patternFormatters).hasSize(1);
512+
final PatternFormatter patternFormatter = patternFormatters.get(0);
513+
514+
// Create the log event and the layout buffer
515+
final LogEvent logEvent = Log4jLogEvent.newBuilder()
516+
.setThrown(exception)
517+
.setLevel(LEVEL)
518+
.build();
519+
final StringBuilder buffer = new StringBuilder(16384);
520+
521+
// Trigger the start latch and format the exception
522+
startLatch.countDown();
523+
startLatch.await();
524+
while (runningThreadCountRef.get() > 0) {
525+
// Give some time slack to increase randomness
526+
LockSupport.parkNanos(1);
527+
patternFormatter.format(logEvent, buffer);
528+
buffer.setLength(0);
483529
}
484530
} finally {
485531
executor.shutdownNow();
486532
}
487533
}
488534

489-
@RepeatedTest(10)
490-
@Issue("https://github.com/apache/logging-log4j2/issues/3929")
491-
void concurrent_suppressed_mutation_should_not_cause_failure() throws Exception {
492-
// Test constants
493-
final int threadCount = Math.max(8, Runtime.getRuntime().availableProcessors());
494-
final ExecutorService executor = Executors.newFixedThreadPool(threadCount);
495-
final Exception exception = new Exception();
496-
final CountDownLatch startLatch =
497-
new CountDownLatch(threadCount + /* the main thread invoking the rendering: */ 1);
498-
final int exceptionPerThreadCount = 100;
499-
final AtomicInteger runningThreadCountRef = new AtomicInteger(threadCount);
500-
501-
// Schedule threads that will start adding suppressed exceptions with the start signal
502-
for (int threadIndex = 0; threadIndex < threadCount; threadIndex++) {
503-
final int threadIndex_ = threadIndex;
504-
executor.submit(() -> {
505-
try {
506-
List<Exception> exceptions = IntStream.range(0, exceptionPerThreadCount)
507-
.mapToObj(exceptionIndex -> new Exception(threadIndex_ + "-" + exceptionIndex))
508-
.collect(Collectors.toList());
509-
startLatch.countDown();
510-
startLatch.await();
511-
for (int exceptionIndex = 0; exceptionIndex < exceptionPerThreadCount; exceptionIndex++) {
512-
exception.addSuppressed(exceptions.get(exceptionIndex));
513-
// Give some time slack to increase randomness
514-
LockSupport.parkNanos(1);
515-
}
516-
} catch (InterruptedException ignored) {
517-
// Restore the interrupt
518-
Thread.currentThread().interrupt();
519-
} finally {
520-
runningThreadCountRef.decrementAndGet();
521-
}
522-
});
523-
}
524-
525-
// Create the formatter
526-
final List<PatternFormatter> patternFormatters = PATTERN_PARSER.parse(patternPrefix, false, true, true);
527-
assertThat(patternFormatters).hasSize(1);
528-
final PatternFormatter patternFormatter = patternFormatters.get(0);
529-
530-
// Create the log event and the layout buffer
531-
final LogEvent logEvent = Log4jLogEvent.newBuilder()
532-
.setThrown(exception)
533-
.setLevel(LEVEL)
534-
.build();
535-
final StringBuilder buffer = new StringBuilder(16384);
536-
537-
// Trigger the start latch and format the exception
538-
startLatch.countDown();
539-
startLatch.await();
540-
while (runningThreadCountRef.get() > 0) {
541-
// Give some time slack to increase randomness
542-
LockSupport.parkNanos(1);
543-
patternFormatter.format(logEvent, buffer);
544-
buffer.setLength(0);
545-
}
535+
private static List<Exception> createExceptionsOfDifferentDepths() {
536+
final StackTraceElement[] stackTrace = new Exception().getStackTrace();
537+
return IntStream.range(0, stackTrace.length)
538+
.mapToObj(depth -> {
539+
final Exception exception = new Exception();
540+
exception.setStackTrace(Arrays.copyOfRange(stackTrace, 0, depth));
541+
return exception;
542+
})
543+
.collect(Collectors.toList());
546544
}
547545
}
548546

0 commit comments

Comments
 (0)