Skip to content

Commit

Permalink
Replace synchronized blocks with Locks (#5565)
Browse files Browse the repository at this point in the history
Signed-off-by: jansupol <[email protected]>
  • Loading branch information
jansupol authored Mar 18, 2024
1 parent 31a6a4d commit 984ba24
Show file tree
Hide file tree
Showing 18 changed files with 298 additions and 142 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -56,6 +56,7 @@
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -92,18 +93,23 @@ private class TypedInstances<TYPE> {
private final MultivaluedMap<TYPE, InstanceContext<?>> singletonInstances = new MultivaluedHashMap<>();
private final ThreadLocal<MultivaluedMap<TYPE, InstanceContext<?>>> threadInstances = new ThreadLocal<>();
private final List<Object> threadPredestroyables = Collections.synchronizedList(new LinkedList<>());
private final ReentrantLock singletonInstancesLock = new ReentrantLock();

private <T> List<InstanceContext<?>> _getSingletons(TYPE clazz) {
List<InstanceContext<?>> si;
synchronized (singletonInstances) {
singletonInstancesLock.lock();
try {
si = singletonInstances.get(clazz);
} finally {
singletonInstancesLock.unlock();
}
return si;
}

@SuppressWarnings("unchecked")
<T> T _addSingleton(TYPE clazz, T instance, Binding<?, ?> binding, Annotation[] qualifiers) {
synchronized (singletonInstances) {
singletonInstancesLock.lock();
try {
// check existing singleton with a qualifier already created by another thread io a meantime
List<InstanceContext<?>> values = singletonInstances.get(clazz);
if (values != null) {
Expand All @@ -118,6 +124,8 @@ <T> T _addSingleton(TYPE clazz, T instance, Binding<?, ?> binding, Annotation[]
singletonInstances.add(clazz, new InstanceContext<>(instance, binding, qualifiers));
threadPredestroyables.add(instance);
return instance;
} finally {
singletonInstancesLock.unlock();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2022 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -37,7 +37,9 @@
import java.util.ResourceBundle;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
Expand Down Expand Up @@ -79,6 +81,7 @@ public final class OsgiRegistry implements SynchronousBundleListener {
private final Map<Long, Map<String, Callable<List<Class<?>>>>> factories =
new HashMap<Long, Map<String, Callable<List<Class<?>>>>>();
private final ReadWriteLock lock = new ReentrantReadWriteLock();
private static final Lock INSTANCE_LOCK = new ReentrantLock();

private static OsgiRegistry instance;

Expand All @@ -90,16 +93,21 @@ public final class OsgiRegistry implements SynchronousBundleListener {
*
* @return an {@code OsgiRegistry} instance.
*/
public static synchronized OsgiRegistry getInstance() {
if (instance == null) {
final ClassLoader classLoader = AccessController
.doPrivileged(ReflectionHelper.getClassLoaderPA(ReflectionHelper.class));
if (classLoader instanceof BundleReference) {
final BundleContext context = FrameworkUtil.getBundle(OsgiRegistry.class).getBundleContext();
if (context != null) { // context could be still null if the current bundle has not been started
instance = new OsgiRegistry(context);
public static OsgiRegistry getInstance() {
INSTANCE_LOCK.lock();
try {
if (instance == null) {
final ClassLoader classLoader = AccessController
.doPrivileged(ReflectionHelper.getClassLoaderPA(ReflectionHelper.class));
if (classLoader instanceof BundleReference) {
final BundleContext context = FrameworkUtil.getBundle(OsgiRegistry.class).getBundleContext();
if (context != null) { // context could be still null if the current bundle has not been started
instance = new OsgiRegistry(context);
}
}
}
} finally {
INSTANCE_LOCK.unlock();
}
return instance;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -33,6 +33,8 @@
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -797,17 +799,20 @@ private void handleClassNotFoundException() throws ServiceConfigurationError {
public abstract static class ServiceIteratorProvider {

private static volatile ServiceIteratorProvider sip;
private static final Object sipLock = new Object();
private static final Lock sipLock = new ReentrantLock();

private static ServiceIteratorProvider getInstance() {
// TODO: check the following is a good practice: Double-check idiom for lazy initialization of fields.
ServiceIteratorProvider result = sip;
if (result == null) { // First check (no locking)
synchronized (sipLock) {
sipLock.lock();
try {
result = sip;
if (result == null) { // Second check (with locking)
sip = result = new DefaultServiceIteratorProvider();
}
} finally {
sipLock.unlock();
}
}
return result;
Expand All @@ -819,8 +824,11 @@ private static void setInstance(final ServiceIteratorProvider sip) throws Securi
final ReflectPermission rp = new ReflectPermission("suppressAccessChecks");
security.checkPermission(rp);
}
synchronized (sipLock) {
sipLock.lock();
try {
ServiceIteratorProvider.sip = sip;
} finally {
sipLock.unlock();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -16,6 +16,9 @@

package org.glassfish.jersey.internal.util.collection;

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

/**
* A collection of {@link Value Value provider} factory & utility methods.
*
Expand Down Expand Up @@ -297,25 +300,28 @@ public T get() {

private static class LazyValueImpl<T> implements LazyValue<T> {

private final Object lock;
private final Lock lock;
private final Value<T> delegate;

private volatile Value<T> value;

public LazyValueImpl(final Value<T> delegate) {
this.delegate = delegate;
this.lock = new Object();
this.lock = new ReentrantLock();
}

@Override
public T get() {
Value<T> result = value;
if (result == null) {
synchronized (lock) {
lock.lock();
try {
result = value;
if (result == null) {
value = result = Values.of(delegate.get());
}
} finally {
lock.unlock();
}
}
return result.get();
Expand Down Expand Up @@ -380,21 +386,22 @@ public static <T, E extends Throwable> LazyUnsafeValue<T, E> lazy(final UnsafeVa

private static class LazyUnsafeValueImpl<T, E extends Throwable> implements LazyUnsafeValue<T, E> {

private final Object lock;
private final Lock lock;
private final UnsafeValue<T, E> delegate;

private volatile UnsafeValue<T, E> value;

public LazyUnsafeValueImpl(final UnsafeValue<T, E> delegate) {
this.delegate = delegate;
this.lock = new Object();
this.lock = new ReentrantLock();
}

@Override
public T get() throws E {
UnsafeValue<T, E> result = value;
if (result == null) {
synchronized (lock) {
lock.lock();
try {
result = value;
//noinspection ConstantConditions
if (result == null) {
Expand All @@ -406,6 +413,8 @@ public T get() throws E {
}
value = result;
}
} finally {
lock.unlock();
}
}
return result.get();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -50,13 +50,7 @@ private HttpDateFormat() {

private static final TimeZone GMT_TIME_ZONE = TimeZone.getTimeZone("GMT");

private static final ThreadLocal<List<SimpleDateFormat>> dateFormats = new ThreadLocal<List<SimpleDateFormat>>() {

@Override
protected synchronized List<SimpleDateFormat> initialValue() {
return createDateFormats();
}
};
private static final ThreadLocal<List<SimpleDateFormat>> dateFormats = ThreadLocal.withInitial(() -> createDateFormats());

private static List<SimpleDateFormat> createDateFormats() {
final SimpleDateFormat[] formats = new SimpleDateFormat[]{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -28,6 +28,8 @@
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import jakarta.ws.rs.core.Cookie;
import jakarta.ws.rs.core.MediaType;
Expand Down Expand Up @@ -622,6 +624,7 @@ private StringListReader() {

private abstract static class ListReader<T> {
private final LRU<String, List<T>> LIST_CACHE = LRU.create();
private final Lock lock = new ReentrantLock();
protected final ListElementCreator<T> creator;

protected ListReader(ListElementCreator<T> creator) {
Expand All @@ -639,7 +642,8 @@ private List<T> readList(final List<T> l, final String header)
List<T> list = LIST_CACHE.getIfPresent(header);

if (list == null) {
synchronized (LIST_CACHE) {
lock.lock();
try {
list = LIST_CACHE.getIfPresent(header);
if (list == null) {
HttpHeaderReader reader = new HttpHeaderReaderImpl(header);
Expand All @@ -655,6 +659,8 @@ private List<T> readList(final List<T> l, final String header)
}
LIST_CACHE.put(header, list);
}
} finally {
lock.unlock();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -22,6 +22,8 @@
import java.lang.reflect.Type;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -252,6 +254,7 @@ private static class UnCloseableInputStream extends InputStream {

private final InputStream original;
private final MessageBodyReader reader;
private final Lock markLock = new ReentrantLock();

private UnCloseableInputStream(final InputStream original, final MessageBodyReader reader) {
this.original = original;
Expand Down Expand Up @@ -284,13 +287,23 @@ public int available() throws IOException {
}

@Override
public synchronized void mark(final int i) {
original.mark(i);
public void mark(final int i) {
markLock.lock();
try {
original.mark(i);
} finally {
markLock.unlock();
}
}

@Override
public synchronized void reset() throws IOException {
original.reset();
public void reset() throws IOException {
markLock.lock();
try {
original.reset();
} finally {
markLock.unlock();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -244,6 +244,7 @@ protected void resume(RequestContext context) {
*/
protected void release(RequestContext context) {
context.release();
currentRequestContext.remove();
}

/**
Expand Down
Loading

0 comments on commit 984ba24

Please sign in to comment.