Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrent connections during a live reload #45481

Closed
FroMage opened this issue Jan 9, 2025 · 2 comments
Closed

Concurrent connections during a live reload #45481

FroMage opened this issue Jan 9, 2025 · 2 comments
Labels
area/devmode kind/bug Something isn't working

Comments

@FroMage
Copy link
Member

FroMage commented Jan 9, 2025

In quarkiverse/quarkus-renarde#266 a user reported an issue which happens when triggering multiple connections during a live reload.

Apparently, according to

we continue serving requests while a live reload is happening:
ClassLoader current = Thread.currentThread().getContextClassLoader();
VertxCoreRecorder.getVertx().get().getOrCreateContext().executeBlocking(new Callable<Boolean>() {
@Override
public Boolean call() {
//the blocking pool may have a stale TCCL
Thread.currentThread().setContextClassLoader(current);
boolean restart = false;
try {
DevConsoleManager.setDoingHttpInitiatedReload(true);
synchronized (this) {
if (nextUpdate < System.currentTimeMillis() || hotReplacementContext.isTest()) {
nextUpdate = System.currentTimeMillis() + HOT_REPLACEMENT_INTERVAL;
Object currentState = VertxHttpRecorder.getCurrentApplicationState();
try {
restart = hotReplacementContext.doScan(true);
} catch (Exception e) {
throw new IllegalStateException("Unable to perform live reload scanning", e);
}
if (currentState != VertxHttpRecorder.getCurrentApplicationState()) {
//its possible a Kafka message or some other source triggered a reload,
//so we could wait for the restart (due to the scan lock)
//but then fail to dispatch to the new application
restart = true;
}
}
}
if (hotReplacementContext.getDeploymentProblem() != null) {
throw new NoStackTraceException(hotReplacementContext.getDeploymentProblem());
}
if (restart) {
//close all connections on close, except for this one
//this prevents long-running requests such as SSE or websockets
//from holding onto the old deployment
Set<ConnectionBase> connections = new HashSet<>(openConnections);
for (ConnectionBase con : connections) {
if (con != connectionBase) {
con.close();
}
}
}
} finally {
DevConsoleManager.setDoingHttpInitiatedReload(false);
}
return restart;
}
}, false).onComplete(new Handler<AsyncResult<Boolean>>() {
@Override
public void handle(AsyncResult<Boolean> event) {
if (event.failed()) {
handleDeploymentProblem(routingContext, event.cause());
} else {
boolean restart = event.result();
if (restart) {
QuarkusExecutorFactory.reinitializeDevModeExecutor();
routingContext.request().headers().set(HEADER_NAME, "true");
VertxHttpRecorder.getRootHandler().handle(routingContext.request());
} else {
routingContext.next();
}
}
}
});

As such, what's happening is that the user makes a change, hits reload, but it's probably taking too long, so they hit reload again, and the second connection is served while the first is still handling the hot reload.

But midway during the request serving, the reload is happening and we're shutting down the Arc Container so the request fails with a really weird and confusing exception.

I could fix the symptom, but that's a waste of time because other applications would fail in different ways.

We could either make new connections (after the hot reload started) wait for the hot reload to be finished, or we could make them serve an error page mentioning that a hot reload is in progress. I suspect the first option is more user-friendly.

To reproduce:

  • Download https://github.com/gbourant/renarde-live-reload-bug
  • Start the app via mvn quarkus:dev
  • Open a browser at http://localhost:8080/hello
  • Edit HelloResource.java and change "Hello from Quarkus REST" (save the file)
  • Go back to the browser, press F5 so it will refresh BUT before it finishes the refresh/live-reload press again F5 fast. (If you just press once the F5 it works as expected)

This should be easy to reproduce with a hot reload test if we make sure the reload takes an artificially long time (not sure how, perhaps a 1s sleep during a custom build step, or a startup bean?).

@famod
Copy link
Member

famod commented Jan 10, 2025

Looks related: #29646 (reproducer: #29646 (comment))

@FroMage
Copy link
Member Author

FroMage commented Jan 10, 2025

Ah, indeed. Same issue. Thanks for noticing. I'll add my findings there then :)

@FroMage FroMage closed this as completed Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devmode kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants