Skip to content

Commit 1d2a955

Browse files
authored
Merge pull request #85 from NamazuStudios/feature/50-jakarta-rs-loader-openapi-context-leak
Fix JakartaRsLoader OpenAPI context leak on element unload
2 parents 5dc5056 + b6742c6 commit 1d2a955

1 file changed

Lines changed: 53 additions & 3 deletions

File tree

deployment-jetty/src/main/java/dev/getelements/elements/deployment/jetty/loader/JakartaRsLoader.java

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import io.swagger.v3.jaxrs2.integration.JaxrsOpenApiContextBuilder;
1010
import io.swagger.v3.jaxrs2.integration.resources.OpenApiResource;
1111
import io.swagger.v3.oas.integration.GenericOpenApiContext;
12+
import io.swagger.v3.oas.integration.OpenApiContextLocator;
1213
import io.swagger.v3.oas.integration.SwaggerConfiguration;
1314
import io.swagger.v3.oas.integration.api.OpenAPIConfiguration;
1415
import io.swagger.v3.oas.integration.api.OpenApiScanner;
@@ -66,6 +67,14 @@ public class JakartaRsLoader implements Loader {
6667
*/
6768
private final Map<Element, Future<?>> pendingMounts = new HashMap<>();
6869

70+
/**
71+
* Tracks the swagger-core OpenAPI context ID minted for each active deployment, keyed by
72+
* element. Needed so {@link #unload} can evict the corresponding entry from
73+
* {@link OpenApiContextLocator}'s static map — see {@link #evictOpenApiContext}. Access is
74+
* guarded by {@link #lock}.
75+
*/
76+
private final Map<Element, String> openApiContextIds = new HashMap<>();
77+
6978
/**
7079
* Daemon thread pool for running the slow Jersey initialisation off the caller's thread.
7180
* A cached pool is used so that multiple elements can start up in parallel without queuing
@@ -279,12 +288,19 @@ private void runMountTask(final Element element, final MountContext ctx) {
279288
try (var mon = Monitor.enter(lock)) {
280289
stillActive = activeDeployments.stream().anyMatch(d -> d.element().equals(element));
281290
pendingMounts.remove(element);
291+
292+
if (!stillActive) {
293+
// unload() already ran (and skipped eviction, since startup was still pending
294+
// at the time) — this task now owns cleanup, including the OpenAPI context.
295+
openApiContextIds.remove(element);
296+
}
282297
}
283298

284299
// 4. Handle failure / post-startup unload.
285300
if (!startSucceeded) {
286301

287302
oaFuture.cancel(true);
303+
evictOpenApiContext(openApiCtxId);
288304

289305
if (stillActive) {
290306
logger.error("Failed to start REST handler for element: {}", elementName, startException);
@@ -300,6 +316,7 @@ private void runMountTask(final Element element, final MountContext ctx) {
300316
if (!stillActive) {
301317

302318
oaFuture.cancel(true);
319+
evictOpenApiContext(openApiCtxId);
303320

304321
logger.info("REST handler for element {} was unloaded during startup; stopping.", elementName);
305322

@@ -431,6 +448,7 @@ public static final class NoScanOpenApiScanner implements OpenApiScanner {
431448
public void unload(final Element element) {
432449

433450
Handler handlerToStop = null;
451+
String openApiCtxId = null;
434452

435453
try (var mon = Monitor.enter(lock)) {
436454

@@ -453,9 +471,9 @@ public void unload(final Element element) {
453471
if (pending != null) {
454472

455473
// Startup is still running on a background thread. Interrupt it (best-effort)
456-
// and let runMountTask() handle stop()/removeHandler() once start() returns.
457-
// Calling stop() here while start() is still executing inside a synchronized
458-
// method would block for the entire startup duration.
474+
// and let runMountTask() handle stop()/removeHandler()/OpenAPI context eviction
475+
// once start() returns. Calling stop() here while start() is still executing
476+
// inside a synchronized method would block for the entire startup duration.
459477
pending.cancel(true);
460478

461479
logger.info("Cancelled pending startup for element {}; background task will clean up.",
@@ -464,6 +482,7 @@ public void unload(final Element element) {
464482
} else {
465483
// Startup is complete — safe to stop synchronously (outside the lock below).
466484
handlerToStop = deployment.handler();
485+
openApiCtxId = openApiContextIds.remove(element);
467486
}
468487
}
469488
}
@@ -478,6 +497,8 @@ public void unload(final Element element) {
478497
} catch (Exception ex) {
479498
logger.error("Failed to cleanly unload REST handler for element: {}",
480499
element.getElementRecord().definition().name(), ex);
500+
} finally {
501+
evictOpenApiContext(openApiCtxId);
481502
}
482503
}
483504
}
@@ -513,6 +534,7 @@ public void load(final PendingDeployment pending, final RuntimeRecord record, fi
513534
// that runMountTask's activeDeployments check correctly reflects the
514535
// current state even if the executor starts the task immediately.
515536
activeDeployments.add(ctx.record());
537+
openApiContextIds.put(element, ctx.openApiCtxId());
516538

517539
final var future = mountExecutor.submit(() -> runMountTask(element, ctx));
518540
pendingMounts.put(element, future);
@@ -537,6 +559,34 @@ private static ServletContextHandler findServletContextHandler(final Handler han
537559
return null;
538560
}
539561

562+
/**
563+
* {@link OpenApiContextLocator} is a process-wide singleton backed by a plain map that only
564+
* exposes {@code get}/{@code put} — swagger-core never removes entries from it. Every
565+
* deployment mints a unique {@code ctxId} (see {@link #deploy}) and registers a
566+
* {@link GenericOpenApiContext} there, which holds the element's {@link Application} and,
567+
* transitively, its classloader. Left alone, that pins the classloader in memory forever, so
568+
* this reflectively removes the entry this loader registered whenever a deployment is
569+
* unloaded, whether or not it finished starting.
570+
*/
571+
private static void evictOpenApiContext(final String openApiCtxId) {
572+
573+
if (openApiCtxId == null) {
574+
return;
575+
}
576+
577+
try {
578+
final var locator = OpenApiContextLocator.getInstance();
579+
final var mapField = OpenApiContextLocator.class.getDeclaredField("map");
580+
mapField.setAccessible(true);
581+
582+
final var map = (Map<?, ?>) mapField.get(locator);
583+
map.remove(openApiCtxId);
584+
} catch (final Exception ex) {
585+
logger.warn("Failed to evict OpenAPI context {} from OpenApiContextLocator; " +
586+
"this may leak the associated element classloader.", openApiCtxId, ex);
587+
}
588+
}
589+
540590
public String getAppOutsideUrl() {
541591
return appOutsideUrl;
542592
}

0 commit comments

Comments
 (0)