Skip to content

Commit 52448e6

Browse files
jansupoljansupol
authored
Fix intermittent premature ClientRuntime finalization (#4508)
* Fix intermittent premature ClientRuntime finalization Signed-off-by: jansupol <[email protected]>
1 parent d11a733 commit 52448e6

File tree

6 files changed

+303
-9
lines changed

6 files changed

+303
-9
lines changed

‎core-client/src/main/java/org/glassfish/jersey/client/ClientConfig.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
33
* Copyright (c) 2018 Payara Foundation and/or its affiliates.
44
*
55
* This program and the accompanying materials are made available under the
@@ -414,12 +414,17 @@ private ClientRuntime initRuntime() {
414414

415415
BootstrapBag bootstrapBag = new ClientBootstrapBag();
416416
bootstrapBag.setManagedObjectsFinalizer(new ManagedObjectsFinalizer(injectionManager));
417-
List<BootstrapConfigurator> bootstrapConfigurators = Arrays.asList(new RequestScope.RequestScopeConfigurator(),
417+
418+
final ClientMessageBodyFactory.MessageBodyWorkersConfigurator messageBodyWorkersConfigurator =
419+
new ClientMessageBodyFactory.MessageBodyWorkersConfigurator();
420+
421+
List<BootstrapConfigurator> bootstrapConfigurators = Arrays.asList(
422+
new RequestScope.RequestScopeConfigurator(),
418423
new ParamConverterConfigurator(),
419424
new ParameterUpdaterConfigurator(),
420425
new RuntimeConfigConfigurator(runtimeCfgState),
421426
new ContextResolverFactory.ContextResolversConfigurator(),
422-
new MessageBodyFactory.MessageBodyWorkersConfigurator(),
427+
messageBodyWorkersConfigurator,
423428
new ExceptionMapperFactory.ExceptionMappersConfigurator(),
424429
new JaxrsProviders.ProvidersConfigurator(),
425430
new AutoDiscoverableConfigurator(RuntimeType.CLIENT));
@@ -455,6 +460,8 @@ private ClientRuntime initRuntime() {
455460
final ClientRuntime crt = new ClientRuntime(configuration, connector, injectionManager, bootstrapBag);
456461

457462
client.registerShutdownHook(crt);
463+
messageBodyWorkersConfigurator.setClientRuntime(crt);
464+
458465
return crt;
459466
}
460467

‎core-client/src/main/java/org/glassfish/jersey/client/ClientMessageBodyFactory.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0, which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the
10+
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
11+
* version 2 with the GNU Classpath Exception, which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
*/
16+
17+
package org.glassfish.jersey.client;
18+
19+
import org.glassfish.jersey.internal.BootstrapBag;
20+
import org.glassfish.jersey.internal.BootstrapConfigurator;
21+
import org.glassfish.jersey.internal.inject.Bindings;
22+
import org.glassfish.jersey.internal.inject.InjectionManager;
23+
import org.glassfish.jersey.internal.inject.InstanceBinding;
24+
import org.glassfish.jersey.internal.util.collection.LazyValue;
25+
import org.glassfish.jersey.internal.util.collection.Value;
26+
import org.glassfish.jersey.internal.util.collection.Values;
27+
import org.glassfish.jersey.message.MessageBodyWorkers;
28+
import org.glassfish.jersey.message.internal.MessageBodyFactory;
29+
30+
import javax.ws.rs.core.Configuration;
31+
32+
class ClientMessageBodyFactory extends MessageBodyFactory {
33+
34+
/**
35+
* Keep reference to {@link ClientRuntime} so that {@code finalize} on it is not called
36+
* before the {@link MessageBodyFactory} is used.
37+
* <p>
38+
* Some entity types {@code @Inject} {@code MessageBodyFactory} for their {@code read} methods,
39+
* but if the finalizer is invoked before that, the HK2 injection manager gets closed.
40+
* </p>
41+
*/
42+
private final LazyValue<ClientRuntime> clientRuntime;
43+
44+
/**
45+
* Create a new message body factory.
46+
*
47+
* @param configuration configuration. Optional - can be null.
48+
* @param clientRuntimeValue - a reference to ClientRuntime.
49+
*/
50+
private ClientMessageBodyFactory(Configuration configuration, Value<ClientRuntime> clientRuntimeValue) {
51+
super(configuration);
52+
clientRuntime = Values.lazy(clientRuntimeValue);
53+
}
54+
55+
/**
56+
* Configurator which initializes and register {@link MessageBodyWorkers} instance into {@link InjectionManager} and
57+
* {@link BootstrapBag}.
58+
*/
59+
static class MessageBodyWorkersConfigurator implements BootstrapConfigurator {
60+
61+
private ClientMessageBodyFactory messageBodyFactory;
62+
private ClientRuntime clientRuntime;
63+
64+
@Override
65+
public void init(InjectionManager injectionManager, BootstrapBag bootstrapBag) {
66+
messageBodyFactory = new ClientMessageBodyFactory(bootstrapBag.getConfiguration(), () -> clientRuntime);
67+
InstanceBinding<ClientMessageBodyFactory> binding =
68+
Bindings.service(messageBodyFactory)
69+
.to(MessageBodyWorkers.class);
70+
injectionManager.register(binding);
71+
}
72+
73+
@Override
74+
public void postInit(InjectionManager injectionManager, BootstrapBag bootstrapBag) {
75+
messageBodyFactory.initialize(injectionManager);
76+
bootstrapBag.setMessageBodyWorkers(messageBodyFactory);
77+
}
78+
79+
void setClientRuntime(ClientRuntime clientRuntime) {
80+
this.clientRuntime = clientRuntime;
81+
}
82+
}
83+
84+
ClientRuntime getClientRuntime() {
85+
return clientRuntime.get();
86+
}
87+
}

‎core-client/src/main/java/org/glassfish/jersey/client/InboundJaxrsResponse.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
33
*
44
* This program and the accompanying materials are made available under the
55
* terms of the Eclipse Public License v. 2.0, which is available at
@@ -22,6 +22,7 @@
2222
import java.util.Locale;
2323
import java.util.Map;
2424
import java.util.Set;
25+
import java.util.concurrent.atomic.AtomicBoolean;
2526

2627
import javax.ws.rs.ProcessingException;
2728
import javax.ws.rs.core.EntityTag;
@@ -51,6 +52,7 @@ class InboundJaxrsResponse extends Response {
5152
private final ClientResponse context;
5253
private final RequestScope scope;
5354
private final RequestContext requestContext;
55+
private final AtomicBoolean isClosed = new AtomicBoolean(false);
5456

5557
/**
5658
* Create new scoped client response.
@@ -139,11 +141,13 @@ public boolean bufferEntity() throws ProcessingException {
139141

140142
@Override
141143
public void close() throws ProcessingException {
142-
try {
143-
context.close();
144-
} finally {
145-
if (requestContext != null) {
146-
requestContext.release();
144+
if (isClosed.compareAndSet(false, true)) {
145+
try {
146+
context.close();
147+
} finally {
148+
if (requestContext != null) {
149+
requestContext.release();
150+
}
147151
}
148152
}
149153
}

‎tests/integration/jersey-4507/pom.xml‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
4+
Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
5+
6+
This program and the accompanying materials are made available under the
7+
terms of the Eclipse Public License v. 2.0, which is available at
8+
http://www.eclipse.org/legal/epl-2.0.
9+
10+
This Source Code may also be made available under the following Secondary
11+
Licenses when the conditions for such availability set forth in the
12+
Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
13+
version 2 with the GNU Classpath Exception, which is available at
14+
https://www.gnu.org/software/classpath/license.html.
15+
16+
SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
17+
18+
-->
19+
<project xmlns="http://maven.apache.org/POM/4.0.0"
20+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
21+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
22+
<parent>
23+
<artifactId>project</artifactId>
24+
<groupId>org.glassfish.jersey.tests.integration</groupId>
25+
<version>2.32.0-SNAPSHOT</version>
26+
</parent>
27+
<modelVersion>4.0.0</modelVersion>
28+
29+
<artifactId>jersey-4507</artifactId>
30+
<dependencies>
31+
<dependency>
32+
<groupId>org.glassfish.jersey.test-framework.providers</groupId>
33+
<artifactId>jersey-test-framework-provider-bundle</artifactId>
34+
<type>pom</type>
35+
<scope>test</scope>
36+
</dependency>
37+
<dependency>
38+
<groupId>org.glassfish.jersey.test-framework</groupId>
39+
<artifactId>jersey-test-framework-core</artifactId>
40+
<scope>test</scope>
41+
</dependency>
42+
<dependency>
43+
<groupId>org.glassfish.jersey.examples</groupId>
44+
<artifactId>server-sent-events-jersey</artifactId>
45+
<version>${project.version}</version>
46+
<scope>test</scope>
47+
</dependency>
48+
</dependencies>
49+
</project>

‎tests/integration/jersey-4507/src/test/java/org/glassfish/jersey/tests/integration/jersey4507/SSETest.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0, which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the
10+
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
11+
* version 2 with the GNU Classpath Exception, which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
*/
16+
17+
package org.glassfish.jersey.tests.integration.jersey4507;
18+
19+
import org.glassfish.jersey.client.ClientConfig;
20+
import org.glassfish.jersey.client.ClientLifecycleListener;
21+
import org.glassfish.jersey.client.ClientProperties;
22+
import org.glassfish.jersey.examples.sse.jersey.App;
23+
import org.glassfish.jersey.examples.sse.jersey.DomainResource;
24+
import org.glassfish.jersey.examples.sse.jersey.ServerSentEventsResource;
25+
import org.glassfish.jersey.media.sse.EventInput;
26+
import org.glassfish.jersey.media.sse.InboundEvent;
27+
import org.glassfish.jersey.media.sse.SseFeature;
28+
import org.glassfish.jersey.server.ResourceConfig;
29+
import org.glassfish.jersey.test.JerseyTest;
30+
import org.junit.Assert;
31+
import org.junit.Test;
32+
33+
import javax.ws.rs.client.Entity;
34+
import javax.ws.rs.core.Application;
35+
import java.util.ArrayList;
36+
import java.util.List;
37+
import java.util.concurrent.Callable;
38+
import java.util.concurrent.CountDownLatch;
39+
import java.util.concurrent.ExecutorService;
40+
import java.util.concurrent.Executors;
41+
import java.util.concurrent.Future;
42+
import java.util.concurrent.TimeUnit;
43+
import java.util.concurrent.atomic.AtomicInteger;
44+
45+
import static org.hamcrest.CoreMatchers.equalTo;
46+
47+
public class SSETest extends JerseyTest {
48+
private static final int MAX_CLIENTS = 10;
49+
private static final int COUNT = 30;
50+
private static final AtomicInteger atomicInteger = new AtomicInteger(0);
51+
private static final CountDownLatch closeLatch = new CountDownLatch(COUNT);
52+
53+
@Override
54+
protected Application configure() {
55+
// enable(TestProperties.LOG_TRAFFIC);
56+
return new ResourceConfig(ServerSentEventsResource.class, DomainResource.class, SseFeature.class);
57+
}
58+
59+
@Override
60+
protected void configureClient(ClientConfig config) {
61+
config.property(ClientProperties.ASYNC_THREADPOOL_SIZE, MAX_CLIENTS + 2);
62+
config.register(new ClientRuntimeCloseVerifier());
63+
}
64+
65+
/**
66+
* Test consuming multiple SSE events sequentially using event input.
67+
*
68+
* @throws Exception in case of a failure during the test execution.
69+
*/
70+
public void testInboundEventReader() throws Exception {
71+
final int MAX_MESSAGES = 5;
72+
final CountDownLatch startLatch = new CountDownLatch(1);
73+
74+
final ExecutorService executor = Executors.newSingleThreadExecutor();
75+
try {
76+
final Future<List<String>> futureMessages =
77+
executor.submit(new Callable<List<String>>() {
78+
79+
@Override
80+
public List<String> call() throws Exception {
81+
final EventInput eventInput = target(App.ROOT_PATH).register(SseFeature.class)
82+
.request().get(EventInput.class);
83+
84+
startLatch.countDown();
85+
86+
final List<String> messages = new ArrayList<String>(MAX_MESSAGES);
87+
try {
88+
for (int i = 0; i < MAX_MESSAGES; i++) {
89+
InboundEvent event = eventInput.read();
90+
messages.add(event.readData());
91+
}
92+
} finally {
93+
if (eventInput != null) {
94+
eventInput.close();
95+
}
96+
}
97+
98+
return messages;
99+
}
100+
});
101+
102+
Assert.assertTrue("Waiting for receiver thread to start has timed out.",
103+
startLatch.await(15000, TimeUnit.SECONDS));
104+
105+
for (int i = 0; i < MAX_MESSAGES; i++) {
106+
target(App.ROOT_PATH).request().post(Entity.text("message " + i));
107+
}
108+
109+
int i = 0;
110+
for (String message : futureMessages.get(50, TimeUnit.SECONDS)) {
111+
Assert.assertThat("Unexpected SSE event data value.", message, equalTo("message " + i++));
112+
}
113+
} finally {
114+
executor.shutdownNow();
115+
}
116+
}
117+
118+
@Test
119+
public void testInboundEventReaderMultiple() throws Exception {
120+
for (int i = 0; i != COUNT; i++) {
121+
testInboundEventReader();
122+
}
123+
124+
System.gc();
125+
closeLatch.await(15_000, TimeUnit.MILLISECONDS);
126+
// One ClientConfig is on the Client
127+
// + COUNT of them is created by .register(SseFeature.class)
128+
Assert.assertEquals(COUNT + 1, atomicInteger.get());
129+
Assert.assertEquals(0, closeLatch.getCount());
130+
}
131+
132+
133+
134+
public static class ClientRuntimeCloseVerifier implements ClientLifecycleListener {
135+
136+
@Override
137+
public void onInit() {
138+
atomicInteger.incrementAndGet();
139+
}
140+
141+
@Override
142+
public void onClose() {
143+
closeLatch.countDown();
144+
}
145+
}
146+
}

‎tests/integration/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
<module>jersey-4003</module>
8686
<module>jersey-4099</module>
8787
<module>jersey-4321</module>
88+
<module>jersey-4507</module>
8889
<module>jetty-response-close</module>
8990
<module>microprofile</module>
9091
<module>portability-jersey-1</module>

0 commit comments

Comments
 (0)