Skip to content

Commit 59800f6

Browse files
authored
[cuebot] Fix channel closing issue (#2274)
The application os not properly closing the grpc channel at shutdown, leaving this SEVERE warning: ``` Channel ManagedChannelImpl{logId=2787, target=elk0815:8444} was not shutdown properly ``` Explanation: `RqdClientGrpc` (cuebot/src/main/java/com/imageworks/spcue/rqd/RqdClientGrpc.java:71-89) holds a Guava `LoadingCache<String, ManagedChannel>`. The `removalListener` calls `conn.shutdown()` **on cache eviction** — but never on application shutdown. The bean has no `destroy-method` (applicationContext-service.xml:39, no destroy hook), and `RqdClientGrpc` has no `shutdown()` method at all. So when cuebot stops: - The cache is GC'd - Each `ManagedChannel` is finalized **without** `shutdown()` having been called - gRPC logs `SEVERE: Channel ... was not shutdown properly!!!` per leaked channel, with the stack capturing where the channel was first allocated (the `RuntimeException: ManagedChannel allocation site` — that's a diagnostic stack, not a real exception) ## LLM usage disclosure Claude Opus was used for investigating the origin of the log and proposing a fix <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved application shutdown procedure to properly clean up network resources and prevent potential resource leaks during shutdown. * Enhanced shutdown sequencing to ensure proper initialization and cleanup order of internal services. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/AcademySoftwareFoundation/OpenCue/pull/2274) <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 21fc1af commit 59800f6

2 files changed

Lines changed: 13 additions & 6 deletions

File tree

cuebot/src/main/java/com/imageworks/spcue/rqd/RqdClientGrpc.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,4 +216,11 @@ public void launchFrame(final RunFrame frame, final VirtualProc proc) {
216216
public void setTestMode(boolean testMode) {
217217
this.testMode = testMode;
218218
}
219+
220+
public void shutdown() {
221+
if (channelCache != null) {
222+
logger.info("Shutting down RqdClientGrpc channel cache");
223+
channelCache.invalidateAll();
224+
}
225+
}
219226
}

cuebot/src/main/resources/conf/spring/applicationContext-service.xml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
<!-- Non-Transactional Service Domain -->
3737
<!-- ##################################################################################### -->
3838

39-
<bean id="rqdClient" class="com.imageworks.spcue.rqd.RqdClientGrpc">
39+
<bean id="rqdClient" class="com.imageworks.spcue.rqd.RqdClientGrpc" destroy-method="shutdown">
4040
<constructor-arg index="0" type="int">
4141
<value>${grpc.rqd_server_port}</value>
4242
</constructor-arg>
@@ -66,7 +66,7 @@
6666
<property name="queueCapacity" value="500" />
6767
</bean>
6868

69-
<bean id="bookingQueue" class="com.imageworks.spcue.dispatcher.BookingQueue" lazy-init="true" destroy-method="shutdown" depends-on="cueDataSource">
69+
<bean id="bookingQueue" class="com.imageworks.spcue.dispatcher.BookingQueue" lazy-init="true" destroy-method="shutdown" depends-on="cueDataSource,rqdClient">
7070
<constructor-arg index="0" type="int">
7171
<value>${booking_queue.threadpool.health_threshold}</value>
7272
</constructor-arg>
@@ -85,7 +85,7 @@
8585
<property name="shutdownDrainMs" value="${healthy_threadpool.shutdown_drain_ms:60000}"/>
8686
</bean>
8787

88-
<bean id="dispatchQueue" class="com.imageworks.spcue.dispatcher.DispatchQueue" destroy-method="shutdown" depends-on="cueDataSource">
88+
<bean id="dispatchQueue" class="com.imageworks.spcue.dispatcher.DispatchQueue" destroy-method="shutdown" depends-on="cueDataSource,rqdClient">
8989
<constructor-arg index="0" type="java.lang.String">
9090
<value>DispatchQueue</value>
9191
</constructor-arg>
@@ -107,7 +107,7 @@
107107
<property name="shutdownDrainMs" value="${healthy_threadpool.shutdown_drain_ms:60000}"/>
108108
</bean>
109109

110-
<bean id="manageQueue" class="com.imageworks.spcue.dispatcher.DispatchQueue" destroy-method="shutdown" depends-on="cueDataSource">
110+
<bean id="manageQueue" class="com.imageworks.spcue.dispatcher.DispatchQueue" destroy-method="shutdown" depends-on="cueDataSource,rqdClient">
111111
<constructor-arg index="0" type="java.lang.String">
112112
<value>ManageQueue</value>
113113
</constructor-arg>
@@ -128,7 +128,7 @@
128128
</constructor-arg>
129129
<property name="shutdownDrainMs" value="${healthy_threadpool.shutdown_drain_ms:60000}"/>
130130
</bean>
131-
<bean id="reportQueue" class="com.imageworks.spcue.dispatcher.HostReportQueue" destroy-method="shutdown" depends-on="cueDataSource">
131+
<bean id="reportQueue" class="com.imageworks.spcue.dispatcher.HostReportQueue" destroy-method="shutdown" depends-on="cueDataSource,rqdClient">
132132
<constructor-arg index="0" type="int">
133133
<value>${report_queue.threadPoolSizeInitial}</value>
134134
</constructor-arg>
@@ -140,7 +140,7 @@
140140
</constructor-arg>
141141
<property name="shutdownDrainMs" value="${host_report_queue.shutdown_drain_ms:60000}"/>
142142
</bean>
143-
<bean id="killQueue" class="com.imageworks.spcue.dispatcher.HostReportQueue" destroy-method="shutdown" depends-on="cueDataSource">
143+
<bean id="killQueue" class="com.imageworks.spcue.dispatcher.HostReportQueue" destroy-method="shutdown" depends-on="cueDataSource,rqdClient">
144144
<constructor-arg index="0" type="int">
145145
<value>${kill_queue.threadPoolSizeInitial}</value>
146146
</constructor-arg>

0 commit comments

Comments
 (0)