Skip to content

Commit

Permalink
Fix another leaks related to CdiComponentProvider cache
Browse files Browse the repository at this point in the history
Signed-off-by: jansupol <jan.supol@oracle.com>
  • Loading branch information
jansupol committed Jun 16, 2022
1 parent d6b7516 commit 80f39d2
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2017 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2022 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 @@ -43,7 +43,7 @@ public class CdiComponentProvider extends ComponentProvider {

private final boolean managerRetrieved;

private final Map<Object, CdiInjectionContext> cdiBeanToContext;
private static final Map<Object, CdiInjectionContext> cdiBeanToContext = new ConcurrentHashMap<Object, CdiInjectionContext>();

/**
* Constructor.
Expand All @@ -53,7 +53,6 @@ public class CdiComponentProvider extends ComponentProvider {
* @throws javax.naming.NamingException when Bean Manager cannot be looked up.
*/
public CdiComponentProvider() throws NamingException {
cdiBeanToContext = new ConcurrentHashMap<Object, CdiInjectionContext>();
InitialContext ic = new InitialContext();
BeanManager manager = null;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2017 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2022 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 @@ -32,7 +32,7 @@
/**
* {@link org.glassfish.tyrus.spi.Writer} implementation used in Servlet integration.
*
* @author Pavel Bucek (pavel.bucek at oracle.com)
* @author Pavel Bucek
*/
class TyrusServletWriter extends Writer implements WriteListener {

Expand Down Expand Up @@ -74,12 +74,17 @@ public TyrusServletWriter(TyrusHttpUpgradeHandler tyrusHttpUpgradeHandler) {
public synchronized void onWritePossible() throws IOException {
LOGGER.log(Level.FINEST, "OnWritePossible called");

while (!queue.isEmpty() && servletOutputStream.isReady()) {
final QueuedFrame queuedFrame = queue.poll();
assert queuedFrame != null;
try /* servletOutputStream.isReady() */ {
while (!queue.isEmpty() && servletOutputStream.isReady()) {
final QueuedFrame queuedFrame = queue.poll();
assert queuedFrame != null;

_write(queuedFrame.dataFrame, queuedFrame.completionHandler);
_write(queuedFrame.dataFrame, queuedFrame.completionHandler);
}
} catch (Exception e) {
onError(e);
}

}

@Override
Expand All @@ -106,16 +111,20 @@ public synchronized void write(final ByteBuffer buffer, CompletionHandler<ByteBu
}
}

if (queue.isEmpty() && servletOutputStream.isReady()) {
_write(buffer, completionHandler);
} else {
final QueuedFrame queuedFrame = new QueuedFrame(completionHandler, buffer);
queue.offer(queuedFrame);
try /* servletOutputStream.isReady() */ {
if (queue.isEmpty() && servletOutputStream.isReady()) {
_write(buffer, completionHandler);
} else {
final QueuedFrame queuedFrame = new QueuedFrame(completionHandler, buffer);
queue.offer(queuedFrame);

if (!isListenerSet) {
isListenerSet = true;
servletOutputStream.setWriteListener(this);
if (!isListenerSet) {
isListenerSet = true;
servletOutputStream.setWriteListener(this);
}
}
} catch (Exception e) {
completionHandler.failed(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2022 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 @@ -161,8 +161,8 @@ private AnnotatedEndpoint(Class<?> annotatedClass, Object instance, ComponentPro
this.endpointEventListener = endpointEventListener;

if (isServerEndpoint) {
if (TyrusServerEndpointConfigurator.class
.equals(((ServerEndpointConfig) configuration).getConfigurator().getClass())) {
final ServerEndpointConfig.Configurator srvEndpointConfig = ((ServerEndpointConfig) configuration).getConfigurator();
if (!TyrusServerEndpointConfigurator.overridesGetEndpointInstance(srvEndpointConfig)) {
// if the platform Configurator is Tyrus provided, it doesn't need to be called to get an endpoint
// instance, since it uses ComponentProviderService anyway.
this.componentProvider = componentProvider;
Expand All @@ -171,8 +171,7 @@ private AnnotatedEndpoint(Class<?> annotatedClass, Object instance, ComponentPro
this.componentProvider = new ComponentProviderService(componentProvider) {
@Override
public <T> Object getEndpointInstance(Class<T> endpointClass) throws InstantiationException {
return ((ServerEndpointConfig) configuration).getConfigurator()
.getEndpointInstance(endpointClass);
return srvEndpointConfig.getEndpointInstance(endpointClass);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,21 +213,13 @@ private TyrusEndpointWrapper(Endpoint endpoint, Class<? extends Endpoint> endpoi
? contextPath.substring(0, contextPath.length() - 1)
: contextPath) + "/"
+ (serverEndpointPath.startsWith("/") ? serverEndpointPath.substring(1) : serverEndpointPath);
// Make sure the provided ComponentProviderService is passed to TyrusServerEndpointConfigurator.
// Otherwise, configurator.getEndpointInstance(endpointClass) let the other CdiComponentProvider
// (configurator.componentProviderService.providers) to instantiate the Endpoint, but
// ComponentProviderService CdiComponentProvider would be called for removeSession() and cleanup
// (different CdiComponentProvider.cdiBeanToContext ==> leak)
if (TyrusServerEndpointConfigurator.class.isInstance(configurator)) {
((TyrusServerEndpointConfigurator) configurator).setComponentProviderService(componentProvider);
}
} else {
this.serverEndpointPath = null;
this.endpointPath = null;
}

this.componentProvider =
configurator == null ? componentProvider : new ComponentProviderService(componentProvider) {
this.componentProvider = !TyrusServerEndpointConfigurator.overridesGetEndpointInstance(configurator)
? componentProvider
: new ComponentProviderService(componentProvider) {
@Override
public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationException {
return configurator.getEndpointInstance(endpointClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@

package org.glassfish.tyrus.core;

import java.lang.reflect.Method;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;

import javax.websocket.Extension;
import javax.websocket.HandshakeResponse;
import javax.websocket.Session;
import javax.websocket.server.HandshakeRequest;
import javax.websocket.server.ServerEndpointConfig;

import org.glassfish.tyrus.core.collection.LazyValue;
import org.glassfish.tyrus.core.collection.Value;
import org.glassfish.tyrus.core.collection.Values;
import org.glassfish.tyrus.core.extension.ExtendedExtension;
import org.glassfish.tyrus.core.frame.Frame;
Expand Down Expand Up @@ -164,10 +164,26 @@ public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationExc
}

/**
* Reuse existing {@link ComponentProviderService}.
* @param componentProviderService The reused {@link ComponentProviderService}.
* Check whether the user defined {@link ServerEndpointConfig.Configurator} has overridden
* {@link ServerEndpointConfig.Configurator#getEndpointInstance(Class)} method.
* In that case, CDIProvider does not manage the instantiation.
* @param configurator The user defined {@link ServerEndpointConfig.Configurator} subclass
* @param <T> The subclass type
* @return {@code true} iff the user creates the endpoint instance on their own.
*/
void setComponentProviderService(ComponentProviderService componentProviderService) {
this.componentProviderService = Values.lazy(() -> componentProviderService);
static <T extends ServerEndpointConfig.Configurator> boolean overridesGetEndpointInstance(T configurator) {
final String getInstanceName = "getEndpointInstance";
if (null == configurator) {
return false;
}
try {
final Method originalMethod = TyrusServerEndpointConfigurator.class.isInstance(configurator)
? TyrusServerEndpointConfigurator.class.getMethod(getInstanceName, Class.class)
: ServerEndpointConfig.Configurator.class.getMethod(getInstanceName, Class.class);
final Method configMethod = configurator.getClass().getMethod(getInstanceName, Class.class);
return !originalMethod.equals(configMethod);
} catch (NoSuchMethodException e) {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package org.glassfish.tyrus.core.collection;

/**
* A collection of {@link Value Value provider} factory & utility methods.
* A collection of {@link Value Value provider} factory &amp; utility methods.
*
* @author Marek Potociar
*/
Expand Down Expand Up @@ -52,12 +52,14 @@ public static <T> Value<T> empty() {
}

/**
* <p>
* Get a new constant {@link Value value provider} whose {@link Value#get() get()}
* method always returns the instance supplied to the {@code value} parameter.
* <p/>
* </p>
* <p>
* In case the supplied value constant is {@code null}, an {@link #empty() empty} value
* provider is returned.
*
* </p>
* @param <T> value type.
* @param value value instance to be provided.
* @return constant value provider.
Expand Down Expand Up @@ -104,10 +106,11 @@ public String toString() {

/**
* Get a new lazily initialized {@link Value value provider}.
* <p/>
* <p>
* The value returned by its {@link Value#get() get()} method is lazily retrieved during a first
* call to the method from the supplied {@code delegate} value provider and is then cached for
* a subsequent retrieval.
* </p>
* <p>
* The implementation of the returned lazy value provider is thread-safe and is guaranteed to
* invoke the {@code get()} method on the supplied {@code delegate} value provider instance at
Expand All @@ -129,9 +132,10 @@ public static <T> LazyValue<T> lazy(final Value<T> delegate) {

/**
* Get a new eagerly initialized {@link Value value provider}.
* <p/>
* <p>
* The value returned by its {@link Value#get() get()} method is eagerly computed from the supplied
* {@code delegate} value provider and is then stored in a final field for a subsequent retrieval.
* </p>
* <p>
* The implementation of the returned eager value provider is thread-safe and is guaranteed to
* invoke the {@code get()} method on the supplied {@code delegate} value provider instance once
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright (c) 2022 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
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
* version 2 with the GNU Classpath Exception, which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
*/

package org.glassfish.tyrus.core;

import org.junit.Assert;
import org.junit.Test;

import javax.websocket.HandshakeResponse;
import javax.websocket.server.HandshakeRequest;
import javax.websocket.server.ServerEndpointConfig;

public class TyrusServerEndpointConfiguratorTest {
@Test
public void overridesGetEndpointInstanceTest() {
ServerEndpointConfig.Configurator config = new TestServerEndpointConfigurator();
Assert.assertFalse(TyrusServerEndpointConfigurator.overridesGetEndpointInstance(config));

config = new TestServerEndpointConfigurator();
Assert.assertFalse(TyrusServerEndpointConfigurator.overridesGetEndpointInstance(config));

config = new TyrusServerEndpointConfigurator();
Assert.assertFalse(TyrusServerEndpointConfigurator.overridesGetEndpointInstance(config));

config = new TyrusServerEndpointConfigurator(){};
Assert.assertFalse(TyrusServerEndpointConfigurator.overridesGetEndpointInstance(config));

config = new TestServerEndpointConfigurator2();
Assert.assertTrue(TyrusServerEndpointConfigurator.overridesGetEndpointInstance(config));
}

private static class TestServerEndpointConfigurator extends ServerEndpointConfig.Configurator {
@Override
public void modifyHandshake(ServerEndpointConfig sec, HandshakeRequest request, HandshakeResponse response) {
super.modifyHandshake(sec, request, response);
}
}

private static class TestServerEndpointConfigurator2 extends ServerEndpointConfig.Configurator {
@Override
public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationException {
return super.getEndpointInstance(endpointClass);
}
}
}

0 comments on commit 80f39d2

Please sign in to comment.