diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/template/TemplateAvailabilityProviders.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/template/TemplateAvailabilityProviders.java index b787dc413a0..de476fe9c2d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/template/TemplateAvailabilityProviders.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/template/TemplateAvailabilityProviders.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.context.ApplicationContext; import org.springframework.core.env.Environment; @@ -52,7 +54,13 @@ public class TemplateAvailabilityProviders { private final Map resolved = new ConcurrentHashMap<>(CACHE_LIMIT); /** - * Map from view name resolve template view, synchronized when accessed. + * Guards access to {@link #cache}. + */ + private final Lock cacheLock = new ReentrantLock(); + + /** + * Map from view name resolve template view, protected by {@link #cacheLock} when + * accessed. */ private final Map cache = new LinkedHashMap<>(CACHE_LIMIT, 0.75f, true) { @@ -133,12 +141,16 @@ public class TemplateAvailabilityProviders { } TemplateAvailabilityProvider provider = this.resolved.get(view); if (provider == null) { - synchronized (this.cache) { + this.cacheLock.lock(); + try { provider = findProvider(view, environment, classLoader, resourceLoader); provider = (provider != null) ? provider : NONE; this.resolved.put(view, provider); this.cache.put(view, provider); } + finally { + this.cacheLock.unlock(); + } } return (provider != NONE) ? provider : null; } diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java index 99ef112ea3b..35a950c6e3b 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -158,9 +158,7 @@ public class FileSystemWatcher { } private void checkNotStarted() { - synchronized (this.monitor) { - Assert.state(this.watchThread == null, "FileSystemWatcher already started"); - } + Assert.state(this.watchThread == null, "FileSystemWatcher already started"); } /** diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/livereload/LiveReloadServer.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/livereload/LiveReloadServer.java index 773fcabc87f..a22216be094 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/livereload/LiveReloadServer.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/livereload/LiveReloadServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -57,7 +59,12 @@ public class LiveReloadServer { private final List connections = new ArrayList<>(); - private final Object monitor = new Object(); + /** + * Guards access to {@link #connections}. + */ + private final Lock connectionsLock = new ReentrantLock(); + + private final Lock lock = new ReentrantLock(); private final int port; @@ -108,7 +115,8 @@ public class LiveReloadServer { * @throws IOException in case of I/O errors */ public int start() throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { Assert.state(!isStarted(), "Server already started"); logger.debug(LogMessage.format("Starting live reload server on port %s", this.port)); this.serverSocket = new ServerSocket(this.port); @@ -119,6 +127,9 @@ public class LiveReloadServer { this.listenThread.start(); return localPort; } + finally { + this.lock.unlock(); + } } /** @@ -126,9 +137,13 @@ public class LiveReloadServer { * @return {@code true} if the server is running */ public boolean isStarted() { - synchronized (this.monitor) { + this.lock.lock(); + try { return this.listenThread != null; } + finally { + this.lock.unlock(); + } } /** @@ -163,7 +178,8 @@ public class LiveReloadServer { * @throws IOException in case of I/O errors */ public void stop() throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.listenThread != null) { closeAllConnections(); try { @@ -184,22 +200,31 @@ public class LiveReloadServer { this.serverSocket = null; } } + finally { + this.lock.unlock(); + } } private void closeAllConnections() throws IOException { - synchronized (this.connections) { + this.connectionsLock.lock(); + try { for (Connection connection : this.connections) { connection.close(); } } + finally { + this.connectionsLock.unlock(); + } } /** * Trigger livereload of all connected clients. */ public void triggerReload() { - synchronized (this.monitor) { - synchronized (this.connections) { + this.lock.lock(); + try { + this.connectionsLock.lock(); + try { for (Connection connection : this.connections) { try { connection.triggerReload(); @@ -209,19 +234,33 @@ public class LiveReloadServer { } } } + finally { + this.connectionsLock.unlock(); + } + } + finally { + this.lock.unlock(); } } private void addConnection(Connection connection) { - synchronized (this.connections) { + this.connectionsLock.lock(); + try { this.connections.add(connection); } + finally { + this.connectionsLock.unlock(); + } } private void removeConnection(Connection connection) { - synchronized (this.connections) { + this.connectionsLock.lock(); + try { this.connections.remove(connection); } + finally { + this.connectionsLock.unlock(); + } } /** diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java index bdc69f02d84..410b269d1d6 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,7 +22,6 @@ import java.lang.reflect.Field; import java.net.URL; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -30,6 +29,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.BlockingDeque; import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.ThreadFactory; @@ -92,7 +92,7 @@ public class Restarter { private final ClassLoaderFiles classLoaderFiles = new ClassLoaderFiles(); - private final Map attributes = new HashMap<>(); + private final Map attributes = new ConcurrentHashMap<>(); private final BlockingDeque leakSafeThreads = new LinkedBlockingDeque<>(); @@ -440,18 +440,11 @@ public class Restarter { } public Object getOrAddAttribute(String name, final ObjectFactory objectFactory) { - synchronized (this.attributes) { - if (!this.attributes.containsKey(name)) { - this.attributes.put(name, objectFactory.getObject()); - } - return this.attributes.get(name); - } + return this.attributes.computeIfAbsent(name, (ignore) -> objectFactory.getObject()); } public Object removeAttribute(String name) { - synchronized (this.attributes) { - return this.attributes.remove(name); - } + return this.attributes.remove(name); } /** diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java index e4a439492d3..3ab00f4ed3a 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import java.nio.channels.AsynchronousCloseException; import java.nio.channels.ServerSocketChannel; import java.nio.channels.SocketChannel; import java.nio.channels.WritableByteChannel; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -49,7 +51,7 @@ public class TunnelClient implements SmartInitializingSingleton { private final TunnelClientListeners listeners = new TunnelClientListeners(); - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final int listenPort; @@ -66,7 +68,8 @@ public class TunnelClient implements SmartInitializingSingleton { @Override public void afterSingletonsInstantiated() { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.serverThread == null) { try { start(); @@ -76,6 +79,9 @@ public class TunnelClient implements SmartInitializingSingleton { } } } + finally { + this.lock.unlock(); + } } /** @@ -84,7 +90,8 @@ public class TunnelClient implements SmartInitializingSingleton { * @throws IOException in case of I/O errors */ public int start() throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { Assert.state(this.serverThread == null, "Server already started"); ServerSocketChannel serverSocketChannel = ServerSocketChannel.open(); serverSocketChannel.socket().bind(new InetSocketAddress(this.listenPort)); @@ -94,6 +101,9 @@ public class TunnelClient implements SmartInitializingSingleton { this.serverThread.start(); return port; } + finally { + this.lock.unlock(); + } } /** @@ -101,7 +111,8 @@ public class TunnelClient implements SmartInitializingSingleton { * @throws IOException in case of I/O errors */ public void stop() throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.serverThread != null) { this.serverThread.close(); try { @@ -113,12 +124,19 @@ public class TunnelClient implements SmartInitializingSingleton { this.serverThread = null; } } + finally { + this.lock.unlock(); + } } protected final ServerThread getServerThread() { - synchronized (this.monitor) { + this.lock.lock(); + try { return this.serverThread; } + finally { + this.lock.unlock(); + } } public void addListener(TunnelClientListener listener) { diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/payload/HttpTunnelPayloadForwarder.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/payload/HttpTunnelPayloadForwarder.java index 0bf486fcaa2..f1a0e40e71d 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/payload/HttpTunnelPayloadForwarder.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/payload/HttpTunnelPayloadForwarder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.io.IOException; import java.nio.channels.WritableByteChannel; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.util.Assert; @@ -36,7 +38,7 @@ public class HttpTunnelPayloadForwarder { private final Map queue = new HashMap<>(); - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final WritableByteChannel targetChannel; @@ -52,7 +54,8 @@ public class HttpTunnelPayloadForwarder { } public void forward(HttpTunnelPayload payload) throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { long seq = payload.getSequence(); if (this.lastRequestSeq != seq - 1) { Assert.state(this.queue.size() < MAXIMUM_QUEUE_SIZE, "Too many messages queued"); @@ -67,6 +70,9 @@ public class HttpTunnelPayloadForwarder { forward(queuedItem); } } + finally { + this.lock.unlock(); + } } } diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java index b03e724dfbd..4d16788699c 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,9 @@ import java.util.Deque; import java.util.Iterator; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -122,7 +125,12 @@ public class HttpTunnelServer { private long disconnectTimeout = DEFAULT_DISCONNECT_TIMEOUT; - private volatile ServerThread serverThread; + /** + * Guards access to {@link #serverThread}. + */ + private final Lock serverThreadLock = new ReentrantLock(); + + private ServerThread serverThread; /** * Creates a new {@link HttpTunnelServer} instance. @@ -164,7 +172,8 @@ public class HttpTunnelServer { * @throws IOException in case of I/O errors */ protected ServerThread getServerThread() throws IOException { - synchronized (this) { + this.serverThreadLock.lock(); + try { if (this.serverThread == null) { ByteChannel channel = this.serverConnection.open(this.longPollTimeout); this.serverThread = new ServerThread(channel); @@ -172,15 +181,22 @@ public class HttpTunnelServer { } return this.serverThread; } + finally { + this.serverThreadLock.unlock(); + } } /** * Called when the server thread exits. */ void clearServerThread() { - synchronized (this) { + this.serverThreadLock.lock(); + try { this.serverThread = null; } + finally { + this.serverThreadLock.unlock(); + } } /** @@ -210,6 +226,13 @@ public class HttpTunnelServer { private final Deque httpConnections; + /** + * Guards access to {@link #httpConnections}. + */ + private final Lock httpConnectionsLock = new ReentrantLock(); + + private final Condition httpConnectionsCondition = this.httpConnectionsLock.newCondition(); + private final HttpTunnelPayloadForwarder payloadForwarder; private boolean closed; @@ -247,7 +270,8 @@ public class HttpTunnelServer { while (this.targetServer.isOpen()) { closeStaleHttpConnections(); ByteBuffer data = HttpTunnelPayload.getPayloadData(this.targetServer); - synchronized (this.httpConnections) { + this.httpConnectionsLock.lock(); + try { if (data != null) { HttpTunnelPayload payload = new HttpTunnelPayload(this.responseSeq.incrementAndGet(), data); payload.logIncoming(); @@ -255,15 +279,20 @@ public class HttpTunnelServer { connection.respond(payload); } } + finally { + this.httpConnectionsLock.unlock(); + } } } private HttpConnection getOrWaitForHttpConnection() { - synchronized (this.httpConnections) { + this.httpConnectionsLock.lock(); + try { HttpConnection httpConnection = this.httpConnections.pollFirst(); while (httpConnection == null) { try { - this.httpConnections.wait(HttpTunnelServer.this.longPollTimeout); + this.httpConnectionsCondition.await(HttpTunnelServer.this.longPollTimeout, + TimeUnit.MILLISECONDS); } catch (InterruptedException ex) { Thread.currentThread().interrupt(); @@ -273,10 +302,14 @@ public class HttpTunnelServer { } return httpConnection; } + finally { + this.httpConnectionsLock.unlock(); + } } private void closeStaleHttpConnections() throws IOException { - synchronized (this.httpConnections) { + this.httpConnectionsLock.lock(); + try { checkNotDisconnected(); Iterator iterator = this.httpConnections.iterator(); while (iterator.hasNext()) { @@ -287,6 +320,9 @@ public class HttpTunnelServer { } } } + finally { + this.httpConnectionsLock.unlock(); + } } private void checkNotDisconnected() { @@ -298,7 +334,8 @@ public class HttpTunnelServer { } private void closeHttpConnections() { - synchronized (this.httpConnections) { + this.httpConnectionsLock.lock(); + try { while (!this.httpConnections.isEmpty()) { try { this.httpConnections.removeFirst().respond(HttpStatus.GONE); @@ -308,6 +345,9 @@ public class HttpTunnelServer { } } } + finally { + this.httpConnectionsLock.unlock(); + } } private void closeTargetServer() { @@ -328,13 +368,17 @@ public class HttpTunnelServer { if (this.closed) { httpConnection.respond(HttpStatus.GONE); } - synchronized (this.httpConnections) { + this.httpConnectionsLock.lock(); + try { while (this.httpConnections.size() > 1) { this.httpConnections.removeFirst().respond(HttpStatus.TOO_MANY_REQUESTS); } this.lastHttpRequestTime = System.currentTimeMillis(); this.httpConnections.addLast(httpConnection); - this.httpConnections.notify(); + this.httpConnectionsCondition.signal(); + } + finally { + this.httpConnectionsLock.unlock(); } forwardToTargetServer(httpConnection); } @@ -368,6 +412,10 @@ public class HttpTunnelServer { private volatile boolean complete = false; + private final Lock lock = new ReentrantLock(); + + private final Condition lockCondition = this.lock.newCondition(); + public HttpConnection(ServerHttpRequest request, ServerHttpResponse response) { this.createTime = System.currentTimeMillis(); this.request = request; @@ -426,8 +474,12 @@ public class HttpTunnelServer { if (this.async == null) { while (!this.complete) { try { - synchronized (this) { - wait(1000); + this.lock.lock(); + try { + this.lockCondition.await(1, TimeUnit.SECONDS); + } + finally { + this.lock.unlock(); } } catch (InterruptedException ex) { @@ -476,9 +528,13 @@ public class HttpTunnelServer { this.async.complete(); } else { - synchronized (this) { + this.lock.lock(); + try { this.complete = true; - notifyAll(); + this.lockCondition.signalAll(); + } + finally { + this.lock.unlock(); } } } diff --git a/spring-boot-project/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/web/servlet/WebDriverScope.java b/spring-boot-project/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/web/servlet/WebDriverScope.java index 5b91ef38e44..f9338002e16 100644 --- a/spring-boot-project/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/web/servlet/WebDriverScope.java +++ b/spring-boot-project/spring-boot-test-autoconfigure/src/main/java/org/springframework/boot/test/autoconfigure/web/servlet/WebDriverScope.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ package org.springframework.boot.test.autoconfigure.web.servlet; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.openqa.selenium.WebDriver; @@ -52,11 +54,17 @@ public class WebDriverScope implements Scope { private static final String[] BEAN_CLASSES = { WEB_DRIVER_CLASS, "org.springframework.test.web.servlet.htmlunit.webdriver.MockMvcHtmlUnitDriverBuilder" }; + /** + * Guards access to {@link #instances}. + */ + private final Lock instancesLock = new ReentrantLock(); + private final Map instances = new HashMap<>(); @Override public Object get(String name, ObjectFactory objectFactory) { - synchronized (this.instances) { + this.instancesLock.lock(); + try { Object instance = this.instances.get(name); if (instance == null) { instance = objectFactory.getObject(); @@ -64,13 +72,20 @@ public class WebDriverScope implements Scope { } return instance; } + finally { + this.instancesLock.unlock(); + } } @Override public Object remove(String name) { - synchronized (this.instances) { + this.instancesLock.lock(); + try { return this.instances.remove(name); } + finally { + this.instancesLock.unlock(); + } } @Override @@ -93,7 +108,8 @@ public class WebDriverScope implements Scope { */ boolean reset() { boolean reset = false; - synchronized (this.instances) { + this.instancesLock.lock(); + try { for (Object instance : this.instances.values()) { reset = true; if (instance instanceof WebDriver webDriver) { @@ -102,6 +118,9 @@ public class WebDriverScope implements Scope { } this.instances.clear(); } + finally { + this.instancesLock.unlock(); + } return reset; } diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/web/SpringBootMockServletContext.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/web/SpringBootMockServletContext.java index ec810bac3fd..30d800a85df 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/web/SpringBootMockServletContext.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/mock/web/SpringBootMockServletContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Files; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.core.io.FileSystemResourceLoader; import org.springframework.core.io.Resource; @@ -44,6 +46,11 @@ public class SpringBootMockServletContext extends MockServletContext { private File emptyRootDirectory; + /** + * Guards access to {@link #emptyRootDirectory}. + */ + private final Lock emptyRootDirectoryLock = new ReentrantLock(); + public SpringBootMockServletContext(String resourceBasePath) { this(resourceBasePath, new FileSystemResourceLoader()); } @@ -91,19 +98,21 @@ public class SpringBootMockServletContext extends MockServletContext { if (resource == null && "/".equals(path)) { // Liquibase assumes that "/" always exists, if we don't have a directory // use a temporary location. + this.emptyRootDirectoryLock.lock(); try { if (this.emptyRootDirectory == null) { - synchronized (this) { - File tempDirectory = Files.createTempDirectory("spr-servlet").toFile(); - tempDirectory.deleteOnExit(); - this.emptyRootDirectory = tempDirectory; - } + File tempDirectory = Files.createTempDirectory("spr-servlet").toFile(); + tempDirectory.deleteOnExit(); + this.emptyRootDirectory = tempDirectory; } return this.emptyRootDirectory.toURI().toURL(); } catch (IOException ex) { // Ignore } + finally { + this.emptyRootDirectoryLock.unlock(); + } } return resource; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/data/RandomAccessDataFile.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/data/RandomAccessDataFile.java index 06d9abcda51..91474edd42b 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/data/RandomAccessDataFile.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/data/RandomAccessDataFile.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.RandomAccessFile; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; /** * {@link RandomAccessData} implementation backed by a {@link RandomAccessFile}. @@ -209,7 +211,7 @@ public class RandomAccessDataFile implements RandomAccessData { private static final class FileAccess { - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final File file; @@ -221,11 +223,15 @@ public class RandomAccessDataFile implements RandomAccessData { } private int read(byte[] bytes, long position, int offset, int length) throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { openIfNecessary(); this.randomAccessFile.seek(position); return this.randomAccessFile.read(bytes, offset, length); } + finally { + this.lock.unlock(); + } } private void openIfNecessary() { @@ -241,20 +247,28 @@ public class RandomAccessDataFile implements RandomAccessData { } private void close() throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.randomAccessFile != null) { this.randomAccessFile.close(); this.randomAccessFile = null; } } + finally { + this.lock.unlock(); + } } private int readByte(long position) throws IOException { - synchronized (this.monitor) { + this.lock.lock(); + try { openIfNecessary(); this.randomAccessFile.seek(position); return this.randomAccessFile.read(); } + finally { + this.lock.unlock(); + } } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/builder/SpringApplicationBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/builder/SpringApplicationBuilder.java index 712caa5e6f1..b479568fcb5 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/builder/SpringApplicationBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/builder/SpringApplicationBuilder.java @@ -76,7 +76,7 @@ public class SpringApplicationBuilder { private final SpringApplication application; - private ConfigurableApplicationContext context; + private volatile ConfigurableApplicationContext context; private SpringApplicationBuilder parent; @@ -145,10 +145,8 @@ public class SpringApplicationBuilder { } configureAsChildIfNecessary(args); if (this.running.compareAndSet(false, true)) { - synchronized (this.running) { - // If not already running copy the sources over and then run. - this.context = build().run(args); - } + // If not already running copy the sources over and then run. + this.context = build().run(args); } return this.context; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/ConfigTreePropertySource.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/ConfigTreePropertySource.java index 866404b4667..1760b240f1a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/ConfigTreePropertySource.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/ConfigTreePropertySource.java @@ -29,6 +29,8 @@ import java.util.EnumSet; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Stream; import org.springframework.boot.convert.ApplicationConversionService; @@ -258,6 +260,11 @@ public class ConfigTreePropertySource extends EnumerablePropertySource imp private final Path path; + /** + * Guards access to {@link #resource}. + */ + private final Lock resourceLock = new ReentrantLock(); + private final Resource resource; private final Origin origin; @@ -341,11 +348,15 @@ public class ConfigTreePropertySource extends EnumerablePropertySource imp } if (this.content == null) { assertStillExists(); - synchronized (this.resource) { + this.resourceLock.lock(); + try { if (this.content == null) { this.content = FileCopyUtils.copyToByteArray(this.resource.getInputStream()); } } + finally { + this.resourceLock.unlock(); + } } return this.content; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/java/SimpleFormatter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/java/SimpleFormatter.java index 1701a6a03a2..fc9dbfbcaf2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/java/SimpleFormatter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/java/SimpleFormatter.java @@ -38,17 +38,15 @@ public class SimpleFormatter extends Formatter { private final String pid = getOrUseDefault(LoggingSystemProperty.PID.getEnvironmentVariableName(), "????"); - private final Date date = new Date(); - @Override - public synchronized String format(LogRecord record) { - this.date.setTime(record.getMillis()); + public String format(LogRecord record) { + Date date = new Date(record.getMillis()); String source = record.getLoggerName(); String message = formatMessage(record); String throwable = getThrowable(record); String thread = getThreadName(); - return String.format(this.format, this.date, source, record.getLoggerName(), - record.getLevel().getLocalizedName(), message, throwable, thread, this.pid); + return String.format(this.format, date, source, record.getLoggerName(), record.getLevel().getLocalizedName(), + message, throwable, thread, this.pid); } private String getThrowable(LogRecord record) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/reactive/ApplicationContextServerWebExchangeMatcher.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/reactive/ApplicationContextServerWebExchangeMatcher.java index 59264b353ff..e3732e219d5 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/reactive/ApplicationContextServerWebExchangeMatcher.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/reactive/ApplicationContextServerWebExchangeMatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.boot.security.reactive; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; import reactor.core.publisher.Mono; @@ -44,7 +46,7 @@ public abstract class ApplicationContextServerWebExchangeMatcher implements S private volatile Supplier context; - private final Object contextLock = new Object(); + private final Lock contextLock = new ReentrantLock(); public ApplicationContextServerWebExchangeMatcher(Class contextClass) { Assert.notNull(contextClass, "Context class must not be null"); @@ -81,13 +83,17 @@ public abstract class ApplicationContextServerWebExchangeMatcher implements S protected Supplier getContext(ServerWebExchange exchange) { if (this.context == null) { - synchronized (this.contextLock) { + this.contextLock.lock(); + try { if (this.context == null) { Supplier createdContext = createContext(exchange); initialized(createdContext); this.context = createdContext; } } + finally { + this.contextLock.unlock(); + } } return this.context; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java index 0f2d9e3858e..3c6542af3fc 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java @@ -16,6 +16,8 @@ package org.springframework.boot.security.servlet; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; import jakarta.servlet.http.HttpServletRequest; @@ -45,7 +47,7 @@ public abstract class ApplicationContextRequestMatcher implements RequestMatc private volatile boolean initialized; - private final Object initializeLock = new Object(); + private final Lock initializeLock = new ReentrantLock(); public ApplicationContextRequestMatcher(Class contextClass) { Assert.notNull(contextClass, "Context class must not be null"); @@ -61,12 +63,16 @@ public abstract class ApplicationContextRequestMatcher implements RequestMatc } Supplier context = () -> getContext(webApplicationContext); if (!this.initialized) { - synchronized (this.initializeLock) { + this.initializeLock.lock(); + try { if (!this.initialized) { initialized(context); this.initialized = true; } } + finally { + this.initializeLock.unlock(); + } } return matches(request, context); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/system/ApplicationTemp.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/system/ApplicationTemp.java index 9c413a0ab70..852db487cd2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/system/ApplicationTemp.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/system/ApplicationTemp.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,8 @@ import java.nio.file.attribute.PosixFilePermissions; import java.security.MessageDigest; import java.util.EnumSet; import java.util.HexFormat; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -51,6 +53,11 @@ public class ApplicationTemp { private volatile Path path; + /** + * Guards access to {@link #path}. + */ + private final Lock pathLock = new ReentrantLock(); + /** * Create a new {@link ApplicationTemp} instance. */ @@ -90,10 +97,14 @@ public class ApplicationTemp { private Path getPath() { if (this.path == null) { - synchronized (this) { + this.pathLock.lock(); + try { String hash = HexFormat.of().withUpperCase().formatHex(generateHash(this.sourceClass)); this.path = createDirectory(getTempDirectory().resolve(hash)); } + finally { + this.pathLock.unlock(); + } } return this.path; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java index f8521016375..a09164b54c4 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java @@ -20,6 +20,8 @@ import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.apache.commons.logging.Log; @@ -57,7 +59,7 @@ public class JettyWebServer implements WebServer { private static final Log logger = LogFactory.getLog(JettyWebServer.class); - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final Server server; @@ -113,21 +115,23 @@ public class JettyWebServer implements WebServer { } private void initialize() { - synchronized (this.monitor) { - try { - // Cache the connectors and then remove them to prevent requests being - // handled before the application context is ready. - this.connectors = this.server.getConnectors(); - JettyWebServer.this.server.setConnectors(null); - // Start the server so that the ServletContext is available - this.server.start(); - this.server.setStopAtShutdown(false); - } - catch (Throwable ex) { - // Ensure process isn't left running - stopSilently(); - throw new WebServerException("Unable to start embedded Jetty web server", ex); - } + this.lock.lock(); + try { + // Cache the connectors and then remove them to prevent requests being + // handled before the application context is ready. + this.connectors = this.server.getConnectors(); + JettyWebServer.this.server.setConnectors(null); + // Start the server so that the ServletContext is available + this.server.start(); + this.server.setStopAtShutdown(false); + } + catch (Throwable ex) { + // Ensure process isn't left running + stopSilently(); + throw new WebServerException("Unable to start embedded Jetty web server", ex); + } + finally { + this.lock.unlock(); } } @@ -142,7 +146,8 @@ public class JettyWebServer implements WebServer { @Override public void start() throws WebServerException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.started) { return; } @@ -179,6 +184,9 @@ public class JettyWebServer implements WebServer { throw new WebServerException("Unable to start embedded Jetty server", ex); } } + finally { + this.lock.unlock(); + } } String getStartedLogMessage() { @@ -241,7 +249,8 @@ public class JettyWebServer implements WebServer { @Override public void stop() { - synchronized (this.monitor) { + this.lock.lock(); + try { this.started = false; if (this.gracefulShutdown != null) { this.gracefulShutdown.abort(); @@ -258,17 +267,22 @@ public class JettyWebServer implements WebServer { throw new WebServerException("Unable to stop embedded Jetty server", ex); } } + finally { + this.lock.unlock(); + } } @Override public void destroy() { - synchronized (this.monitor) { - try { - this.server.stop(); - } - catch (Exception ex) { - throw new WebServerException("Unable to destroy embedded Jetty server", ex); - } + this.lock.lock(); + try { + this.server.stop(); + } + catch (Exception ex) { + throw new WebServerException("Unable to destroy embedded Jetty server", ex); + } + finally { + this.lock.unlock(); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java index 9290aeada33..8eb9b17dd8e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java @@ -20,6 +20,8 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import javax.naming.NamingException; @@ -60,7 +62,7 @@ public class TomcatWebServer implements WebServer { private static final AtomicInteger containerCounter = new AtomicInteger(-1); - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final Map serviceConnectors = new HashMap<>(); @@ -106,41 +108,43 @@ public class TomcatWebServer implements WebServer { private void initialize() throws WebServerException { logger.info("Tomcat initialized with " + getPortsDescription(false)); - synchronized (this.monitor) { + this.lock.lock(); + try { + addInstanceIdToEngineName(); + + Context context = findContext(); + context.addLifecycleListener((event) -> { + if (context.equals(event.getSource()) && Lifecycle.START_EVENT.equals(event.getType())) { + // Remove service connectors so that protocol binding doesn't + // happen when the service is started. + removeServiceConnectors(); + } + }); + + // Start the server to trigger initialization listeners + this.tomcat.start(); + + // We can re-throw failure exception directly in the main thread + rethrowDeferredStartupExceptions(); + try { - addInstanceIdToEngineName(); - - Context context = findContext(); - context.addLifecycleListener((event) -> { - if (context.equals(event.getSource()) && Lifecycle.START_EVENT.equals(event.getType())) { - // Remove service connectors so that protocol binding doesn't - // happen when the service is started. - removeServiceConnectors(); - } - }); - - // Start the server to trigger initialization listeners - this.tomcat.start(); - - // We can re-throw failure exception directly in the main thread - rethrowDeferredStartupExceptions(); - - try { - ContextBindings.bindClassLoader(context, context.getNamingToken(), getClass().getClassLoader()); - } - catch (NamingException ex) { - // Naming is not enabled. Continue - } - - // Unlike Jetty, all Tomcat threads are daemon threads. We create a - // blocking non-daemon to stop immediate shutdown - startDaemonAwaitThread(); + ContextBindings.bindClassLoader(context, context.getNamingToken(), getClass().getClassLoader()); } - catch (Exception ex) { - stopSilently(); - destroySilently(); - throw new WebServerException("Unable to start embedded Tomcat", ex); + catch (NamingException ex) { + // Naming is not enabled. Continue } + + // Unlike Jetty, all Tomcat threads are daemon threads. We create a + // blocking non-daemon to stop immediate shutdown + startDaemonAwaitThread(); + } + catch (Exception ex) { + stopSilently(); + destroySilently(); + throw new WebServerException("Unable to start embedded Tomcat", ex); + } + finally { + this.lock.unlock(); } } @@ -205,7 +209,8 @@ public class TomcatWebServer implements WebServer { @Override public void start() throws WebServerException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.started) { return; } @@ -233,6 +238,9 @@ public class TomcatWebServer implements WebServer { ContextBindings.unbindClassLoader(context, context.getNamingToken(), getClass().getClassLoader()); } } + finally { + this.lock.unlock(); + } } String getStartedLogMessage() { @@ -324,7 +332,8 @@ public class TomcatWebServer implements WebServer { @Override public void stop() throws WebServerException { - synchronized (this.monitor) { + this.lock.lock(); + try { boolean wasStarted = this.started; try { this.started = false; @@ -342,6 +351,9 @@ public class TomcatWebServer implements WebServer { } } } + finally { + this.lock.unlock(); + } } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java index bce6152a7e6..0ac3b76ff2f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java @@ -25,6 +25,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import io.undertow.Undertow; import io.undertow.server.HttpHandler; @@ -63,7 +65,7 @@ public class UndertowWebServer implements WebServer { private final AtomicReference gracefulShutdownCallback = new AtomicReference<>(); - private final Object monitor = new Object(); + private final Lock lock = new ReentrantLock(); private final Undertow.Builder builder; @@ -104,7 +106,8 @@ public class UndertowWebServer implements WebServer { @Override public void start() throws WebServerException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (this.started) { return; } @@ -136,6 +139,9 @@ public class UndertowWebServer implements WebServer { } } } + finally { + this.lock.unlock(); + } } private void destroySilently() { @@ -268,7 +274,8 @@ public class UndertowWebServer implements WebServer { @Override public void stop() throws WebServerException { - synchronized (this.monitor) { + this.lock.lock(); + try { if (!this.started) { return; } @@ -286,6 +293,9 @@ public class UndertowWebServer implements WebServer { throw new WebServerException("Unable to stop Undertow", ex); } } + finally { + this.lock.unlock(); + } } @Override