Skip to content

Commit 9953c14

Browse files
reinhapareinhapa
committed
Adds missing executors shutdown
Signed-off-by: Patrick Reinhart <[email protected]>
1 parent c013777 commit 9953c14

File tree

3 files changed

+180
-14
lines changed

3 files changed

+180
-14
lines changed

‎exist-core-jcstress/dependency-reduced-pom.xml‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
3+
<parent>
4+
<artifactId>exist-parent</artifactId>
5+
<groupId>org.exist-db</groupId>
6+
<version>6.1.0-SNAPSHOT</version>
7+
<relativePath>../exist-parent/pom.xml</relativePath>
8+
</parent>
9+
<modelVersion>4.0.0</modelVersion>
10+
<artifactId>exist-core-jcstress</artifactId>
11+
<name>eXist-db Core JCStress Tests</name>
12+
<build>
13+
<plugins>
14+
<plugin>
15+
<artifactId>maven-shade-plugin</artifactId>
16+
<executions>
17+
<execution>
18+
<id>main</id>
19+
<phase>package</phase>
20+
<goals>
21+
<goal>shade</goal>
22+
</goals>
23+
<configuration>
24+
<finalName>${uberjar.name}</finalName>
25+
<filters>
26+
<filter>
27+
<artifact>*:*</artifact>
28+
<excludes>
29+
<exclude>META-INF/*.SF</exclude>
30+
<exclude>META-INF/*.DSA</exclude>
31+
<exclude>META-INF/*.RSA</exclude>
32+
</excludes>
33+
</filter>
34+
</filters>
35+
<transformers>
36+
<transformer>
37+
<mainClass>org.openjdk.jcstress.Main</mainClass>
38+
</transformer>
39+
<transformer>
40+
<resource>META-INF/TestList</resource>
41+
</transformer>
42+
</transformers>
43+
</configuration>
44+
</execution>
45+
</executions>
46+
</plugin>
47+
</plugins>
48+
</build>
49+
<properties>
50+
<uberjar.name>jcstress</uberjar.name>
51+
</properties>
52+
</project>

‎exist-core-jmh/dependency-reduced-pom.xml‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
3+
<parent>
4+
<artifactId>exist-parent</artifactId>
5+
<groupId>org.exist-db</groupId>
6+
<version>6.1.0-SNAPSHOT</version>
7+
<relativePath>../exist-parent/pom.xml</relativePath>
8+
</parent>
9+
<modelVersion>4.0.0</modelVersion>
10+
<artifactId>exist-core-jmh</artifactId>
11+
<name>eXist-db Core JMH benchmarks</name>
12+
<description>JMH Benchmarks for exist-core</description>
13+
<build>
14+
<plugins>
15+
<plugin>
16+
<groupId>com.mycila</groupId>
17+
<artifactId>license-maven-plugin</artifactId>
18+
<configuration>
19+
<licenseSets>
20+
<licenseSet>
21+
<header>${project.parent.relativePath}/LGPL-21-license.template.txt</header>
22+
<excludes>
23+
<exclude>src/main/java/org/exist/storage/lock/LockTableBenchmark.java</exclude>
24+
<exclude>src/main/java/org/exist/xquery/utils/StringJoinBenchmark.java</exclude>
25+
<exclude>src/main/java/org/exist/xquery/utils/URIUtilsBenchmark.java</exclude>
26+
</excludes>
27+
</licenseSet>
28+
<licenseSet>
29+
<header>${project.parent.relativePath}/FDB-backport-LGPL-21-ONLY-license.template.txt</header>
30+
<includes>
31+
<include>src/main/java/org/exist/storage/lock/LockTableBenchmark.java</include>
32+
<include>src/main/java/org/exist/xquery/utils/StringJoinBenchmark.java</include>
33+
<include>src/main/java/org/exist/xquery/utils/URIUtilsBenchmark.java</include>
34+
</includes>
35+
</licenseSet>
36+
</licenseSets>
37+
</configuration>
38+
</plugin>
39+
<plugin>
40+
<artifactId>maven-dependency-plugin</artifactId>
41+
<executions>
42+
<execution>
43+
<id>analyze</id>
44+
<goals>
45+
<goal>analyze-only</goal>
46+
</goals>
47+
<configuration>
48+
<failOnWarning>true</failOnWarning>
49+
<ignoredUnusedDeclaredDependencies>
50+
<ignoredUnusedDeclaredDependency>org.openjdk.jmh:jmh-generator-annprocess:jar:${jmh.version}</ignoredUnusedDeclaredDependency>
51+
</ignoredUnusedDeclaredDependencies>
52+
</configuration>
53+
</execution>
54+
</executions>
55+
</plugin>
56+
<plugin>
57+
<artifactId>maven-compiler-plugin</artifactId>
58+
<configuration>
59+
<compilerVersion>${project.build.target}</compilerVersion>
60+
<source>${project.build.source}</source>
61+
<target>${project.build.target}</target>
62+
</configuration>
63+
</plugin>
64+
<plugin>
65+
<artifactId>maven-shade-plugin</artifactId>
66+
<executions>
67+
<execution>
68+
<phase>package</phase>
69+
<goals>
70+
<goal>shade</goal>
71+
</goals>
72+
<configuration>
73+
<finalName>${project.artifactId}-${project.version}-benchmarks</finalName>
74+
<transformers>
75+
<transformer>
76+
<mainClass>org.openjdk.jmh.Main</mainClass>
77+
</transformer>
78+
</transformers>
79+
<filters>
80+
<filter>
81+
<artifact>*:*</artifact>
82+
<excludes>
83+
<exclude>META-INF/*.SF</exclude>
84+
<exclude>META-INF/*.DSA</exclude>
85+
<exclude>META-INF/*.RSA</exclude>
86+
</excludes>
87+
</filter>
88+
</filters>
89+
</configuration>
90+
</execution>
91+
</executions>
92+
</plugin>
93+
</plugins>
94+
</build>
95+
<dependencies>
96+
<dependency>
97+
<groupId>org.openjdk.jmh</groupId>
98+
<artifactId>jmh-generator-annprocess</artifactId>
99+
<version>1.35</version>
100+
<scope>provided</scope>
101+
</dependency>
102+
</dependencies>
103+
<properties>
104+
<jmh.version>1.35</jmh.version>
105+
<uberjar.name>benchmarks</uberjar.name>
106+
</properties>
107+
</project>

‎exist-core/src/test/java/org/exist/storage/BrokerPoolTest.java‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void noPrivilegeEscalationThroughBrokerRelease() throws EXistException {
5252
//take a broker with the guest user
5353
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
5454
final Subject guestUser = pool.getSecurityManager().getGuestSubject();
55-
try(final DBBroker broker1 = pool.get(Optional.of(guestUser))) {
55+
try (final DBBroker broker1 = pool.get(Optional.of(guestUser))) {
5656

5757
assertEquals("Expected `guest` user, but was: " + broker1.getCurrentSubject().getName(), guestUser.getId(), broker1.getCurrentSubject().getId());
5858

@@ -72,7 +72,7 @@ public void privilegeStableWhenSubjectNull() throws EXistException {
7272
//take a broker with the SYSTEM user
7373
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
7474
final Subject sysUser = pool.getSecurityManager().getSystemSubject();
75-
try(final DBBroker broker1 = pool.get(Optional.of(sysUser))) {
75+
try (final DBBroker broker1 = pool.get(Optional.of(sysUser))) {
7676

7777
assertEquals("Expected `SYSTEM` user, but was: " + broker1.getCurrentSubject().getName(), sysUser.getId(), broker1.getCurrentSubject().getId());
7878

@@ -90,7 +90,7 @@ public void privilegeStableWhenSubjectNull() throws EXistException {
9090
public void guestDefaultPriviledge() throws EXistException {
9191
//take a broker with default perms
9292
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
93-
try(final DBBroker broker1 = pool.getBroker()) {
93+
try (final DBBroker broker1 = pool.getBroker()) {
9494

9595
final Subject guestUser = pool.getSecurityManager().getGuestSubject();
9696

@@ -111,7 +111,7 @@ public void noPrivilegeEscalationThroughBrokerRelease_xmldb() throws EXistExcept
111111
//take a broker with the guest user
112112
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
113113
final Subject guestUser = pool.getSecurityManager().getGuestSubject();
114-
try(final DBBroker broker1 = pool.get(Optional.of(guestUser))) {
114+
try (final DBBroker broker1 = pool.get(Optional.of(guestUser))) {
115115

116116
assertEquals("Expected `guest` user, but was: " + broker1.getCurrentSubject().getName(), guestUser.getId(), broker1.getCurrentSubject().getId());
117117

@@ -179,15 +179,15 @@ public void canReleaseWhenSaturated() throws InterruptedException, ExecutionExce
179179
executor.shutdown();
180180
} finally {
181181
// release all brokers from brokerUsers
182-
if(firstBrokerReleaseLatch.getCount() == 1) {
182+
if (firstBrokerReleaseLatch.getCount() == 1) {
183183
firstBrokerReleaseLatch.countDown();
184184
}
185185
releaseLatch.countDown();
186186
assertTrue(executor.awaitTermination(1, TimeUnit.SECONDS));
187187
for (Future<Void> task : tasks) {
188188
task.get();
189189
}
190-
for (Runnable task: executor.shutdownNow()) {
190+
for (Runnable task : executor.shutdownNow()) {
191191
assertNotNull(task);
192192
}
193193
}
@@ -197,32 +197,38 @@ public void canReleaseWhenSaturated() throws InterruptedException, ExecutionExce
197197
public void concurrentShutdownAndUse() throws InterruptedException, ExecutionException {
198198
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
199199
final int maxBrokers = pool.getMax();
200+
final int taskAmount = maxBrokers * 50;
200201

201202
// test requires at least 5 leasedBrokers to prove the issue
202203
assertTrue(maxBrokers > 4);
203204

204205
final CountDownLatch readyLatch = new CountDownLatch(1);
205206
final CountDownLatch executeLatch = new CountDownLatch(1);
206-
final ExecutorService executor = Executors.newFixedThreadPool(maxBrokers);
207-
final List<Future<Void>> tasks = new ArrayList<>(maxBrokers);
207+
final ExecutorService executor = Executors.newFixedThreadPool(taskAmount);
208+
final List<Future<Void>> tasks = new ArrayList<>(taskAmount);
208209
final Consumer<BrokerPool> brokerAquire = brokerPool -> {
209210
try (final DBBroker broker = brokerPool.getBroker()) {
211+
TimeUnit.SECONDS.sleep(1);
210212
} catch (EXistException e) {
211-
throw new RuntimeException(e);
213+
throw new IllegalStateException(e);
214+
} catch (InterruptedException e) {
215+
Thread.currentThread().interrupt();
216+
throw new IllegalStateException(e);
212217
}
213218
};
214-
for (int count=0; count < maxBrokers; count++) {
215-
tasks.add(executor.submit(new PoolAction(pool, readyLatch, executeLatch, count == 0 ? BrokerPool::shutdown: brokerAquire)));
219+
for (int count = 0; count < taskAmount; count++) {
220+
tasks.add(executor.submit(new PoolAction(pool, readyLatch, executeLatch, (count % 2 == 0) ? BrokerPool::shutdown : brokerAquire)));
216221
}
222+
executor.shutdown();
217223

218-
TimeUnit.SECONDS.sleep(1);
224+
TimeUnit.SECONDS.sleep(2);
219225
readyLatch.countDown();
220226

221227
assertTrue(executor.awaitTermination(1, TimeUnit.MINUTES));
222228
for (Future<Void> task : tasks) {
223229
task.get();
224230
}
225-
for (Runnable task: executor.shutdownNow()) {
231+
for (Runnable task : executor.shutdownNow()) {
226232
assertNotNull(task);
227233
}
228234
}
@@ -239,6 +245,7 @@ static class PoolAction implements Callable<Void> {
239245
this.excuteLatch = excuteLatch;
240246
this.action = action;
241247
}
248+
242249
@Override
243250
public Void call() throws InterruptedException {
244251
readyLatch.await();
@@ -261,7 +268,7 @@ public BrokerUser(final BrokerPool brokerPool, final CountDownLatch acquiredLatc
261268

262269
@Override
263270
public Void call() throws EXistException, InterruptedException {
264-
try(final DBBroker broker = brokerPool.getBroker()) {
271+
try (final DBBroker broker = brokerPool.getBroker()) {
265272

266273
// signal that we have acquired the broker
267274
acquiredLatch.countDown();

0 commit comments

Comments
 (0)