Merge pull request #881 from cqse/ts/45056_class_hot_reload_coverage_issue

TS-45056 Class hot reloading detected and coverage dump fixed

Author: DreierF

CHANGELOG.md

@@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [fix] _agent_: Fixed `IllegalStateException: Can't add different class with same name` when application servers like JBoss perform a reload without full restart or when duplicate class files exist across multiple archives
 - [feature] _teamscale-gradle-plugin_: Annotated tasks with `@DisableCachingByDefault` where caching can't be applied
 
 # 36.4.0

CLAUDE.md

@@ -51,7 +51,7 @@ Entry point: `PreMain.premain()` in `agent/src/main/java/com/teamscale/jacoco/ag
 4. Classes are instrumented at load time with coverage probes
 
 **Coverage modes:**
-- **Interval-based** (`Agent` class) - Periodically dumps coverage (default every 10 min)
+- **Interval-based** (`Agent` class) - Periodically dumps coverage (default every 480 min)
 - **Test-wise** (`TestwiseCoverageAgent` class) - Per-test coverage via HTTP endpoints (`/test/start`, `/test/end`)
 
 **Key classes:**

README.md

@@ -11,6 +11,86 @@
 * [Teamscale Gradle Plugin](https://docs.teamscale.com/reference/integrations/gradle-plugin/)
 * [Teamscale Maven Plugin](https://docs.teamscale.com/reference/integrations/maven-plugin/)
 
+## Architecture
+
+The profiler is a JVM agent that uses [JaCoCo](https://www.jacoco.org/) under the hood for bytecode instrumentation and coverage recording. 
+The diagram below shows the data flow from JVM startup to coverage upload.
+
+```
+┌──────────────────────────────────────────────────────────────────────────┐
+│  JVM                                                                     │
+│                                                                          │
+│  -javaagent:teamscale-jacoco-agent.jar=...                               │
+│       │                                                                  │
+│       ▼                                                                  │
+│  ┌──────────┐     ┌─────────────────────┐     ┌──────────────────────┐   │
+│  │ PreMain  │────▶│ JaCoCoPreMain       │────▶│ LenientCoverage-     │   │
+│  │          │     │                     │     │ Transformer          │   │
+│  │ Parses   │     │ Creates JaCoCo      │     │                      │   │
+│  │ options, │     │ runtime, registers  │     │ Registered with JVM  │   │
+│  │ logging  │     │ class transformer   │     │ via addTransformer() │   │
+│  └──────────┘     └─────────────────────┘     └──────────┬───────────┘   │
+│       │                                                  │               │
+│       │           ┌──────────────────────────────────────────────────┐   │
+│       │           │  Class Loading                                   │   │
+│       │           │                                                  │   │
+│       │           │  For every class loaded by the JVM:              │   │
+│       │           │  1. Transformer receives original bytecode       │   │
+│       │           │  2. JaCoCo injects boolean[] probes at branches  │   │
+│       │           │     and lines                                    │   │
+│       │           │  3. Modified bytecode returned to JVM            │   │
+│       │           │                                                  │   │
+│       │           │  Instrumentation happens ONLY at class load      │   │
+│       │           │  time. Already-loaded classes are never          │   │
+│       │           │  retransformed.                                  │   │
+│       │           └──────────────────────────────────────────────────┘   │
+│       │                                                  │               │
+│       │           ┌──────────────────────────────────────────────────┐   │
+│       │           │  Runtime                                         │   │
+│       │           │                                                  │   │
+│       │           │  Probes fire during normal code execution.       │   │
+│       │           │  Each probe sets a flag in a per-class           │   │
+│       │           │  boolean[], tracking which lines/branches ran.   │   │
+│       │           │  Data accumulates in JaCoCo runtime memory.      │   │
+│       │           └──────────────────────────────────────────────────┘   │
+│       │                                                                  │
+│       ▼                                                                  │
+│  ┌─────────────────────────────────────────────────────────────────┐     │
+│  │  Agent (Normal mode)         OR   TestwiseCoverageAgent         │     │
+│  │                                                                 │     │
+│  │  HTTP server (Jetty + Jersey) for control:                      │     │
+│  │    POST /dump ─── trigger coverage dump                         │     │
+│  │    POST /test/start, /test/end ─── per-test coverage            │     │
+│  └─────────────────────────────────────────────────────────────────┘     │
+│                                                                          │
+└──────────────────────────────────────────────────────────────────────────┘
+         │
+         │  Periodically (default: every 480 min) or on HTTP request
+         ▼
+┌─────────────────────────────────────────────────────────────────────────┐
+│  Coverage Dump Pipeline                                                 │
+│                                                                         │
+│  1. JacocoRuntimeController.dumpAndReset()                              │
+│     Retrieves execution data from JaCoCo runtime and resets probes.     │
+│                                                                         │
+│  2. JaCoCoXmlReportGenerator.convertSingleDumpToReport()                │
+│     Reads class files (from auto-created dump dir or user-specified     │
+│     class-dir), matches them with execution data by CRC64 class ID,     │
+│     and produces a JaCoCo XML coverage report.                          │
+│                                                                         │
+│  3. IUploader.upload()                                                  │
+│     Sends the XML report to the configured destination.                 │
+│                                                                         │
+└────────────────────────────┬────────────────────────────────────────────┘
+                             │
+              ┌──────────────┼──────────────┐
+              ▼              ▼              ▼
+        ┌──────────┐  ┌───────────┐  ┌────────────┐
+        │Teamscale │  │Artifactory│  │Local disk  │
+        │  Server  │  │ / Azure   │  │            │
+        └──────────┘  └───────────┘  └────────────┘
+```
+
 ## Development
 
 Before starting development, please enable the pre-commit hook by running:

agent/src/main/java/com/teamscale/jacoco/agent/Agent.java

@@ -40,9 +40,6 @@
  */
 public class Agent extends AgentBase {
 
-	/** Converts binary data to XML. */
-	private final JaCoCoXmlReportGenerator generator;
-
 	/** Regular dump task. */
 	private Timer timer;
 
@@ -57,9 +54,6 @@ public Agent(AgentOptions options, Instrumentation instrumentation)
 		uploader = options.createUploader(instrumentation);
 		logger.info("Upload method: {}", uploader.describe());
 		retryUnsuccessfulUploads(options, uploader);
-		generator = new JaCoCoXmlReportGenerator(options.getClassDirectoriesOrZips(),
-				options.getLocationIncludeFilter(), options.getDuplicateClassFileBehavior(),
-				options.shouldIgnoreUncoveredClasses(), wrap(logger));
 
 		if (options.shouldDumpInIntervals()) {
 			timer = new Timer(this::dumpReport, Duration.ofMinutes(options.getDumpIntervalInMinutes()));
@@ -165,9 +159,11 @@ private static void deleteDirectoryIfEmpty(Path directory) throws IOException {
 	/**
 	 * Dumps the current execution data, converts it, writes it to the output directory defined in {@link #options} and
 	 * uploads it if an uploader is configured. Logs any errors, never throws an exception.
+	 * <p>
+	 * Synchronized because this can be triggered concurrently by the timer and by the HTTP /dump endpoint.
 	 */
 	@Override
-	public void dumpReport() {
+	public synchronized void dumpReport() {
 		logger.debug("Starting dump");
 
 		try {
@@ -188,6 +184,10 @@ private void dumpReportUnsafe() {
 			return;
 		}
 
+		JaCoCoXmlReportGenerator generator = new JaCoCoXmlReportGenerator(
+				options.getClassDirectoriesOrZips(), options.getLocationIncludeFilter(),
+				options.getDuplicateClassFileBehavior(), options.shouldIgnoreUncoveredClasses(), wrap(logger));
+
 		try (Benchmark ignored = new Benchmark("Generating the XML report")) {
 			File outputFile = options.createNewFileInOutputDirectory("jacoco", "xml");
 			CoverageFile coverageFile = generator.convertSingleDumpToReport(dump, outputFile);

agent/src/main/java/com/teamscale/jacoco/agent/options/JacocoAgentOptionsBuilder.java

@@ -35,11 +35,11 @@ public String createJacocoAgentOptions() throws AgentOptionParseException, IOExc
 		// Don't dump class files in testwise mode when coverage is written to an exec file
 		boolean needsClassFiles = agentOptions.mode == EMode.NORMAL || agentOptions.testwiseCoverageMode != ETestwiseCoverageMode.EXEC_FILE;
 		if (agentOptions.classDirectoriesOrZips.isEmpty() && needsClassFiles) {
-			Path tempDir = createTemporaryDumpDirectory();
-			tempDir.toFile().deleteOnExit();
-			builder.append(",classdumpdir=").append(tempDir.toAbsolutePath());
+			Path classDumpDirectory = createTemporaryDumpDirectory();
+			classDumpDirectory.toFile().deleteOnExit();
+			builder.append(",classdumpdir=").append(classDumpDirectory.toAbsolutePath());
 
-			agentOptions.classDirectoriesOrZips = Collections.singletonList(tempDir.toFile());
+			agentOptions.classDirectoriesOrZips = Collections.singletonList(classDumpDirectory.toFile());
 		}
 
 		agentOptions.additionalJacocoOptions

report-generator/src/main/kotlin/com/teamscale/report/compact/CompactCoverageReportGenerator.kt

@@ -30,15 +30,16 @@ class CompactCoverageReportGenerator(
 	duplicateClassFileBehavior,
 	true,
 	logger,
-	TeamscaleCompactCoverageBuilder()
+	{ TeamscaleCompactCoverageBuilder() }
 ) {
 
 	/** Creates an XML report based on the given session and coverage data.  */
 	@Throws(IOException::class)
 	override fun createReport(
 		output: OutputStream,
 		sessionInfo: SessionInfo?,
-		store: ExecutionDataStore
+		store: ExecutionDataStore,
+		coverageVisitor: TeamscaleCompactCoverageBuilder
 	) {
 		val compactReportData = coverageVisitor.buildReport()
 		compactReportData.checkForEmptyReport()

report-generator/src/main/kotlin/com/teamscale/report/jacoco/JaCoCoBasedReportGenerator.kt

@@ -17,7 +17,14 @@ import java.io.OutputStream
 /**
  * Base class for generating reports based on the binary JaCoCo exec dump files.
  *
- * It takes care of ignoring non-identical classes with the same fully qualified name and classes without coverage.
+ * JaCoCo's execution data only contains per-class boolean arrays indicating which probes fired. It does not contain
+ * any structural information (method names, line numbers, branch locations). The class files in
+ * [codeDirectoriesOrArchives] provide that structural information: JaCoCo re-analyzes them to reconstruct which probes
+ * correspond to which lines and branches, then merges this with the execution data to produce a meaningful coverage
+ * report.
+ *
+ * Since the same class can appear in multiple archives (for example, a dependency bundled in two WARs), this class
+ * detects such duplicates via [EnhancedCoverageVisitor] and handles them according to [duplicateClassFileBehavior].
  *
  * @param codeDirectoriesOrArchives Directories and zip files that contain class files.
  * @param locationIncludeFilter Include filter to apply to all locations during class file traversal.
@@ -30,8 +37,16 @@ abstract class JaCoCoBasedReportGenerator<Visitor : ICoverageVisitor>(
 	private val duplicateClassFileBehavior: EDuplicateClassFileBehavior,
 	private val ignoreUncoveredClasses: Boolean,
 	private val logger: ILogger,
-	/** The coverage visitor which will be called with all the data found in the exec files. */
-	protected val coverageVisitor: Visitor,
+	/**
+	 * Supplier that creates a fresh coverage visitor for each dump. This is a supplier rather than
+	 * a plain instance because [EnhancedCoverageVisitor] and the coverage visitor must share the
+	 * same lifecycle: both are created per [convertSingleDumpToReport] call. If the coverage visitor
+	 * were reused across dumps, it would still carry class IDs from a previous dump, and a class
+	 * that reappears with a different CRC64 (for example, after an application server hot-reload)
+	 * would crash inside JaCoCo's CoverageBuilder instead of being handled by our duplicate
+	 * detection in [EnhancedCoverageVisitor].
+	 */
+	private val coverageVisitorSupplier: () -> Visitor,
 ) {
 
 	/**
@@ -43,9 +58,10 @@ abstract class JaCoCoBasedReportGenerator<Visitor : ICoverageVisitor>(
 	fun convertSingleDumpToReport(dump: Dump, outputFilePath: File): CoverageFile {
 		val coverageFile = CoverageFile(outputFilePath)
 		val mergedStore = dump.store
-		analyzeStructureAndAnnotateCoverage(mergedStore)
+		val coverageVisitor = coverageVisitorSupplier()
+		analyzeStructureAndAnnotateCoverage(mergedStore, coverageVisitor)
 		coverageFile.outputStream.use { outputStream ->
-			createReport(outputStream, dump.info, mergedStore)
+			createReport(outputStream, dump.info, mergedStore, coverageVisitor)
 		}
 		return coverageFile
 	}
@@ -62,27 +78,48 @@ abstract class JaCoCoBasedReportGenerator<Visitor : ICoverageVisitor>(
 		convertSingleDumpToReport(Dump(sessionInfo, loader.executionDataStore), outputFilePath)
 	}
 
-	/** Creates an XML report based on the given session and coverage data.  */
+	/**
+	 * Creates an XML report based on the given session and coverage data.
+	 *
+	 * @param coverageVisitor The visitor that was populated during [analyzeStructureAndAnnotateCoverage] for this dump.
+	 *   Passed as a parameter (rather than being a field) because a fresh instance is created per dump via
+	 *   [coverageVisitorSupplier].
+	 */
 	@Throws(IOException::class)
 	protected abstract fun createReport(
 		output: OutputStream,
 		sessionInfo: SessionInfo?,
-		store: ExecutionDataStore
+		store: ExecutionDataStore,
+		coverageVisitor: Visitor
 	)
 
 	/**
-	 * Analyzes the structure of the class files in [.codeDirectoriesOrArchives] and builds an in-memory coverage
+	 * Analyzes the structure of the class files in [codeDirectoriesOrArchives] and builds an in-memory coverage
 	 * report with the coverage in the given store.
+	 *
+	 * We share a single [EnhancedCoverageVisitor] across all entries so that its duplicate-detection map tracks classes
+	 * globally. Without this, the same class appearing in two different archives (for example, old and new WAR after an
+	 * application server reload) would bypass our duplicate handling and hit `CoverageBuilder` directly, which always
+	 * throws `IllegalStateException` regardless of the configured [duplicateClassFileBehavior].
 	 */
 	@Throws(IOException::class)
-	private fun analyzeStructureAndAnnotateCoverage(store: ExecutionDataStore) {
+	private fun analyzeStructureAndAnnotateCoverage(store: ExecutionDataStore, coverageVisitor: Visitor) {
+		val visitor = EnhancedCoverageVisitor(coverageVisitor)
 		codeDirectoriesOrArchives.forEach { file ->
-			FilteringAnalyzer(store, EnhancedCoverageVisitor(), locationIncludeFilter, logger)
+			FilteringAnalyzer(store, visitor, locationIncludeFilter, logger)
 				.analyzeAll(file)
 		}
 	}
 
-	private inner class EnhancedCoverageVisitor : ICoverageVisitor {
+	/**
+	 * Wrapper around the actual coverage visitor (typically JaCoCo's [org.jacoco.core.analysis.CoverageBuilder] or
+	 * [com.teamscale.report.compact.TeamscaleCompactCoverageBuilder]) that intercepts duplicate, non-identical class
+	 * files before they reach the wrapped visitor. Without this, duplicates would hit `CoverageBuilder` directly,
+	 * which always throws `IllegalStateException` regardless of the configured [duplicateClassFileBehavior].
+	 */
+	private inner class EnhancedCoverageVisitor(
+		private val coverageVisitor: Visitor
+	) : ICoverageVisitor {
 
 		private val classIdByClassName: MutableMap<String, Long> = mutableMapOf()
 
@@ -95,6 +132,7 @@ abstract class JaCoCoBasedReportGenerator<Visitor : ICoverageVisitor>(
 				warnAboutDuplicateClassFile(coverage)
 				return
 			}
+
 			coverageVisitor.visitCoverage(coverage)
 		}
 

report-generator/src/main/kotlin/com/teamscale/report/jacoco/JaCoCoXmlReportGenerator.kt

@@ -34,15 +34,16 @@ class JaCoCoXmlReportGenerator(
 	duplicateClassFileBehavior,
 	ignoreUncoveredClasses,
 	logger,
-	CoverageBuilder()
+	{ CoverageBuilder() }
 ) {
 
 	/** Creates an XML report based on the given session and coverage data.  */
 	@Throws(IOException::class)
 	override fun createReport(
 		output: OutputStream,
 		sessionInfo: SessionInfo?,
-		store: ExecutionDataStore
+		store: ExecutionDataStore,
+		coverageVisitor: CoverageBuilder
 	) {
 		val bundleCoverage = BundleCoverageImpl("dummybundle", emptyList(), coverageVisitor.sourceFiles)
 		bundleCoverage.checkForEmptyReport()