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

Inconsistent stop behavior across various Web Server implementations #44144

Open
nosan opened this issue Feb 6, 2025 · 2 comments
Open

Inconsistent stop behavior across various Web Server implementations #44144

nosan opened this issue Feb 6, 2025 · 2 comments
Labels
for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged

Comments

@nosan
Copy link
Contributor

nosan commented Feb 6, 2025

Javadoc states:

Stops the web server. Calling this method on an already stopped server has no effect.

I reviewed all WebServer implementations and identified some inconsistencies among them.

In the case of TomcatWebServer, it consistently tries to removeServiceConnectors, and
stop method can be invoked multiple times.

public void stop() throws WebServerException {
	synchronized (this.monitor) {
		boolean wasStarted = this.started;
		try {
			this.started = false;
			if (this.gracefulShutdown != null) {
				this.gracefulShutdown.abort();
			}
			removeServiceConnectors();
		}
		catch (Exception ex) {
			throw new WebServerException("Unable to stop embedded Tomcat", ex);
		}
		finally {
			if (wasStarted) {
				containerCounter.decrementAndGet();
			}
		}
	}
}

For NettyWebServer, if an error occurs, it silently catches the exception without
throwing a WebServerException. Additionally, subsequent calls to stop() will have no
effect.

    public void stop() throws WebServerException {
	if (this.disposableServer != null) {
		if (this.gracefulShutdown != null) {
			this.gracefulShutdown.abort();
		}
		try {
			if (this.lifecycleTimeout != null) {
				this.disposableServer.disposeNow(this.lifecycleTimeout);
			}
			else {
				this.disposableServer.disposeNow();
			}
		}
		catch (IllegalStateException ex) {
			// Continue
		}
		this.disposableServer = null;
	}
}

Similarly to Tomcat, JettyWebServer attempts to stop its connectors and allows the stop
method to be invoked multiple times.

public void stop() {
	synchronized (this.monitor) {
		this.started = false;
		if (this.gracefulShutdown != null) {
			this.gracefulShutdown.abort();
		}
		try {
			for (Connector connector : this.server.getConnectors()) {
				connector.stop();
			}
		}
		catch (InterruptedException ex) {
			Thread.currentThread().interrupt();
		}
		catch (Exception ex) {
			throw new WebServerException("Unable to stop embedded Jetty server", ex);
		}
	}
}

In contrast, UndertowWebServer makes the second call to stop() ineffective, preventing any retries to stop the
WebServer in case of failure.

@Override
public void stop() throws WebServerException {
	synchronized (this.monitor) {
		if (!this.started) {
			return;
		}
		this.started = false;
		if (this.gracefulShutdown != null) {
			notifyGracefulCallback(false);
		}
		try {
			this.undertow.stop();
			for (Closeable closeable : this.closeables) {
				closeable.close();
			}
		}
		catch (Exception ex) {
			throw new WebServerException("Unable to stop Undertow", ex);
		}
	}
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 6, 2025
@JayakrishnanJS
Copy link

@nosan could u pls assign this issue to me

@wilkinsona
Copy link
Member

Thanks for the offer, @JayakrishnanJS, but this will have to be discussed by the core team to decide what, if anything, we want to do.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

5 participants