Skip to content

Change "synchronized" to reentrant lock for virtual-threads #1188

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

Merged
merged 2 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023-2023 the original author or authors.
* Copyright 2023-2024 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.
Expand All @@ -20,6 +20,7 @@
import java.nio.charset.StandardCharsets;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.concurrent.locks.ReentrantLock;

import com.microsoft.azure.functions.ExecutionContext;
import com.microsoft.azure.functions.HttpMethod;
Expand Down Expand Up @@ -48,6 +49,7 @@
*
* @author Christian Tzolov
* @author Oleg Zhurakousky
* @author Omer Celik
*
*/
public class AzureWebProxyInvoker implements FunctionInstanceInjector {
Expand All @@ -62,6 +64,8 @@ public class AzureWebProxyInvoker implements FunctionInstanceInjector {

private ServletContext servletContext;

private static final ReentrantLock globalLock = new ReentrantLock();

@SuppressWarnings("unchecked")
@Override
public <T> T getInstance(Class<T> functionClass) throws Exception {
Expand All @@ -72,13 +76,20 @@ public <T> T getInstance(Class<T> functionClass) throws Exception {
/**
* Because the getInstance is called by Azure Java Function on every function request we need to cache the Spring
* context initialization on the first function call.
* Double-Checked Locking Optimization was used to avoid unnecessary locking overhead.
* @throws ServletException error.
*/
private void initialize() throws ServletException {
synchronized (AzureWebProxyInvoker.class.getName()) {
if (mvc == null) {
Class<?> startClass = FunctionClassUtils.getStartClass();
this.mvc = ServerlessMVC.INSTANCE(startClass);
if (mvc == null) {
try {
globalLock.lock();
if (mvc == null) {
Class<?> startClass = FunctionClassUtils.getStartClass();
this.mvc = ServerlessMVC.INSTANCE(startClass);
}
}
finally {
globalLock.unlock();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021-2022 the original author or authors.
* Copyright 2021-2024 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.
Expand All @@ -17,6 +17,7 @@
package org.springframework.cloud.function.adapter.azure;

import java.util.Map;
import java.util.concurrent.locks.ReentrantLock;

import com.microsoft.azure.functions.spi.inject.FunctionInstanceInjector;
import org.apache.commons.logging.Log;
Expand All @@ -37,6 +38,7 @@
* hook. The Azure Java Worker delegates scans the classpath for service definition and delegates the function class
* creation to this instance factory.
* @author Christian Tzolov
* @author Omer Celik
* @since 3.2.9
*/
public class AzureFunctionInstanceInjector implements FunctionInstanceInjector {
Expand All @@ -45,6 +47,8 @@ public class AzureFunctionInstanceInjector implements FunctionInstanceInjector {

private static ConfigurableApplicationContext APPLICATION_CONTEXT;

private static final ReentrantLock globalLock = new ReentrantLock();

/**
* This method is called by the Azure Java Worker on every function invocation. The Worker sends in the classes
* annotated with @FunctionName annotations and allows the Spring framework to initialize the function instance as a
Expand Down Expand Up @@ -83,13 +87,20 @@ public <T> T getInstance(Class<T> functionClass) throws Exception {

/**
* Create a static Application Context instance shared between multiple function invocations.
* Double-Checked Locking Optimization was used to avoid unnecessary locking overhead.
*/
private static void initialize() {
synchronized (AzureFunctionInstanceInjector.class.getName()) {
if (APPLICATION_CONTEXT == null) {
Class<?> springConfigurationClass = FunctionClassUtils.getStartClass();
logger.info("Initializing: " + springConfigurationClass);
APPLICATION_CONTEXT = springApplication(springConfigurationClass).run();
if (APPLICATION_CONTEXT == null) {
try {
globalLock.lock();
if (APPLICATION_CONTEXT == null) {
Class<?> springConfigurationClass = FunctionClassUtils.getStartClass();
logger.info("Initializing: " + springConfigurationClass);
APPLICATION_CONTEXT = springApplication(springConfigurationClass).run();
}
}
finally {
globalLock.unlock();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021-2022 the original author or authors.
* Copyright 2021-2024 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.
Expand All @@ -25,6 +25,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.microsoft.azure.functions.ExecutionContext;
Expand Down Expand Up @@ -66,6 +67,7 @@
* @author Oleg Zhurakousky
* @author Chris Bono
* @author Christian Tzolov
* @author Omer Celik
*
* @since 3.2
*
Expand All @@ -85,6 +87,8 @@ public class FunctionInvoker<I, O> {

private static JsonMapper OBJECT_MAPPER;

private static final ReentrantLock globalLock = new ReentrantLock();

public FunctionInvoker(Class<?> configurationClass) {
try {
initialize(configurationClass);
Expand Down Expand Up @@ -355,30 +359,38 @@ private MessageHeaders getHeaders(HttpRequestMessage<I> event) {
return new MessageHeaders(headers);
}

/**
* Double-Checked Locking Optimization was used to avoid unnecessary locking overhead.
*/
private static void initialize(Class<?> configurationClass) {
synchronized (FunctionInvoker.class.getName()) {
if (FUNCTION_CATALOG == null) {
logger.info("Initializing: " + configurationClass);
SpringApplication builder = springApplication(configurationClass);
APPLICATION_CONTEXT = builder.run();

Map<String, FunctionCatalog> mf = APPLICATION_CONTEXT.getBeansOfType(FunctionCatalog.class);
if (CollectionUtils.isEmpty(mf)) {
OBJECT_MAPPER = new JacksonMapper(new ObjectMapper());
JsonMessageConverter jsonConverter = new JsonMessageConverter(OBJECT_MAPPER);
SmartCompositeMessageConverter messageConverter = new SmartCompositeMessageConverter(
if (FUNCTION_CATALOG == null) {
try {
globalLock.lock();
if (FUNCTION_CATALOG == null) {
logger.info("Initializing: " + configurationClass);
SpringApplication builder = springApplication(configurationClass);
APPLICATION_CONTEXT = builder.run();

Map<String, FunctionCatalog> mf = APPLICATION_CONTEXT.getBeansOfType(FunctionCatalog.class);
if (CollectionUtils.isEmpty(mf)) {
OBJECT_MAPPER = new JacksonMapper(new ObjectMapper());
JsonMessageConverter jsonConverter = new JsonMessageConverter(OBJECT_MAPPER);
SmartCompositeMessageConverter messageConverter = new SmartCompositeMessageConverter(
Collections.singletonList(jsonConverter));
FUNCTION_CATALOG = new SimpleFunctionRegistry(
FUNCTION_CATALOG = new SimpleFunctionRegistry(
APPLICATION_CONTEXT.getBeanFactory().getConversionService(),
messageConverter, OBJECT_MAPPER);
}
else {
OBJECT_MAPPER = APPLICATION_CONTEXT.getBean(JsonMapper.class);
FUNCTION_CATALOG = mf.values().iterator().next();
}
else {
OBJECT_MAPPER = APPLICATION_CONTEXT.getBean(JsonMapper.class);
FUNCTION_CATALOG = mf.values().iterator().next();
}
}
}
finally {
globalLock.unlock();
}
}

}

private static SpringApplication springApplication(Class<?> configurationClass) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023-2023 the original author or authors.
* Copyright 2023-2024 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.
Expand All @@ -19,6 +19,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.locks.ReentrantLock;

import jakarta.servlet.AsyncContext;
import jakarta.servlet.AsyncEvent;
Expand All @@ -39,6 +40,7 @@
* Implementation of Async context for {@link ServerlessMVC}.
*
* @author Oleg Zhurakousky
* @author Omer Celik
*/
public class ServerlessAsyncContext implements AsyncContext {
private final HttpServletRequest request;
Expand All @@ -55,6 +57,8 @@ public class ServerlessAsyncContext implements AsyncContext {

private final List<Runnable> dispatchHandlers = new ArrayList<>();

private final ReentrantLock globalLock = new ReentrantLock();


public ServerlessAsyncContext(ServletRequest request, @Nullable ServletResponse response) {
this.request = (HttpServletRequest) request;
Expand All @@ -64,14 +68,18 @@ public ServerlessAsyncContext(ServletRequest request, @Nullable ServletResponse

public void addDispatchHandler(Runnable handler) {
Assert.notNull(handler, "Dispatch handler must not be null");
synchronized (this) {
try {
this.globalLock.lock();
if (this.dispatchedPath == null) {
this.dispatchHandlers.add(handler);
}
else {
handler.run();
}
}
finally {
this.globalLock.unlock();
}
}

@Override
Expand Down Expand Up @@ -102,10 +110,14 @@ public void dispatch(String path) {

@Override
public void dispatch(@Nullable ServletContext context, String path) {
synchronized (this) {
try {
this.globalLock.lock();
this.dispatchedPath = path;
this.dispatchHandlers.forEach(Runnable::run);
}
finally {
this.globalLock.unlock();
}
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.logging.Log;
Expand All @@ -39,6 +40,7 @@

/**
* @author Oleg Zhurakousky
* @author Omer Celik
*/
public final class JsonMasker {

Expand All @@ -50,22 +52,41 @@ public final class JsonMasker {

private final Set<String> keysToMask;

private static final ReentrantLock globalLock = new ReentrantLock();

private JsonMasker() {
this.keysToMask = loadKeys();
this.mapper = new JacksonMapper(new ObjectMapper());

}

public synchronized static JsonMasker INSTANCE() {
/**
* Double-Checked Locking Optimization was used to avoid unnecessary locking overhead.
*/
public static JsonMasker INSTANCE() {
if (jsonMasker == null) {
jsonMasker = new JsonMasker();
try {
globalLock.lock();
if (jsonMasker == null) {
jsonMasker = new JsonMasker();
}
}
finally {
globalLock.unlock();
}
}
return jsonMasker;
}

public synchronized static JsonMasker INSTANCE(Set<String> keysToMask) {
INSTANCE().addKeys(keysToMask);
return jsonMasker;
public static JsonMasker INSTANCE(Set<String> keysToMask) {
try {
globalLock.lock();
INSTANCE().addKeys(keysToMask);
return jsonMasker;
}
finally {
globalLock.unlock();
}
}

public String[] getKeysToMask() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023-2023 the original author or authors.
* Copyright 2023-2024 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.
Expand All @@ -17,6 +17,7 @@
package org.springframework.cloud.function.integration.dsl;

import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
Expand All @@ -28,6 +29,7 @@
* The helper class to lookup functions from the catalog in lazy manner and cache their instances.
*
* @author Artem Bilan
* @author Omer Celik
*
* @since 4.0.3
*/
Expand Down Expand Up @@ -72,16 +74,21 @@ private <T> T requireNonNull(Class<?> functionType, String functionDefinition)
*/
private static <T> Supplier<T> memoize(Supplier<? extends T> delegate) {
AtomicReference<T> value = new AtomicReference<>();
ReentrantLock lock = new ReentrantLock();
return () -> {
T val = value.get();
if (val == null) {
synchronized (value) {
try {
lock.lock();
val = value.get();
if (val == null) {
val = delegate.get();
value.set(val);
}
}
finally {
lock.unlock();
}
}
return val;
};
Expand Down