This is an automated email from the ASF dual-hosted git repository.
lokiore pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push:
new e0289913b1 PHOENIX-7965 :- Guard
ConnectionQueryServicesImpl.getMetaDataCache() against a closed connection
(#2580)
e0289913b1 is described below
commit e0289913b16c5c8a4339b2e72d8a7e823f804d0d
Author: Lokesh Khurana <[email protected]>
AuthorDate: Wed Jul 29 12:04:48 2026 -0700
PHOENIX-7965 :- Guard ConnectionQueryServicesImpl.getMetaDataCache()
against a closed connection (#2580)
getMetaDataCache() returned the volatile latestMetaData field directly,
with no
null guard. When a connection is closed concurrently with query compilation
(for example connection pool teardown or cluster failover), close() sets
latestMetaData = null. An in-flight compile then reaches the cache via
PhoenixConnection.getTableRef()/getFunction()/getSchema(), which call
getMetaDataCache().getTableRef(key) (and friends) on the null return,
producing
a bare, causeless NullPointerException with no message.
Every other cache accessor/mutator on this class (addTable, removeTable,
pruneTables, the internal getTable path, etc.) already calls
throwConnectionClosedIfNullMetaData() first, which throws a descriptive
IllegalStateException("Connection to the cluster is closed").
getMetaDataCache()
was the one accessor missing that guard. This change adds the same guard so
a
closed connection surfaces the descriptive exception instead of a bare NPE.
No caller relies on a null return: every production caller of
ConnectionQueryServices.getMetaDataCache() immediately dereferences the
result,
so this only upgrades the failure mode from an opaque NPE to a descriptive
exception; it does not change behavior on an open connection.
Tests: added unit tests asserting getMetaDataCache() throws the descriptive
IllegalStateException when the cache has been nulled (closed-connection
state),
and returns the cache unchanged when present.
Generated-by: Claude Code (Opus 4.8 (1M context))
---
.../phoenix/query/ConnectionQueryServicesImpl.java | 11 ++++-
.../query/ConnectionQueryServicesImplTest.java | 54 ++++++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletion(-)
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 2c27a57fd3..f14ae7aa71 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -1090,7 +1090,16 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices
}
public PMetaData getMetaDataCache() {
- return latestMetaData;
+ // A concurrent close() (e.g. connection pool teardown or cluster
failover) nulls out
+ // latestMetaData. Read it once into a local so the null-check and the
returned reference
+ // are guaranteed to be the same value; a caller that immediately
dereferences the result
+ // then surfaces the descriptive "connection closed" exception instead of
a bare NPE,
+ // matching every other cache accessor/mutator on this class.
+ PMetaData cache = latestMetaData;
+ if (cache == null) {
+ throwConnectionClosedException();
+ }
+ return cache;
}
@Override
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
index a6a55a5081..af03a2b0f4 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
@@ -73,6 +73,7 @@ import org.apache.phoenix.exception.PhoenixIOException;
import org.apache.phoenix.jdbc.ConnectionInfo;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.monitoring.GlobalClientMetrics;
+import org.apache.phoenix.schema.PMetaData;
import org.apache.phoenix.util.ReadOnlyProps;
import org.junit.Before;
import org.junit.ClassRule;
@@ -408,4 +409,57 @@ public class ConnectionQueryServicesImplTest {
verify(mockAdmin,
Mockito.times(1)).deleteTable(TableName.valueOf("TEST_TABLE"));
verify(mockConn).getAdmin();
}
+
+ /**
+ * When a connection is closed concurrently with query compilation (e.g.
connection pool teardown
+ * or cluster failover), the metadata cache is nulled out.
getMetaDataCache() must surface the
+ * descriptive "connection closed" exception rather than returning null and
letting callers hit a
+ * bare NPE.
+ */
+ @Test
+ public void testGetMetaDataCacheThrowsWhenClosed()
+ throws NoSuchFieldException, IllegalAccessException {
+ ConnectionQueryServicesImpl cqsi = newRealCqsi();
+ // Simulate the post-close() state where close() has set latestMetaData =
null.
+ setLatestMetaData(cqsi, null);
+ try {
+ cqsi.getMetaDataCache();
+ fail("Expected IllegalStateException for a closed connection, but no
exception was thrown");
+ } catch (IllegalStateException e) {
+ assertEquals("Connection to the cluster is closed", e.getMessage());
+ }
+ }
+
+ /**
+ * When the metadata cache is present (open connection), getMetaDataCache()
returns it unchanged.
+ */
+ @Test
+ public void testGetMetaDataCacheReturnsCacheWhenOpen()
+ throws NoSuchFieldException, IllegalAccessException {
+ ConnectionQueryServicesImpl cqsi = newRealCqsi();
+ PMetaData metaData = Mockito.mock(PMetaData.class);
+ setLatestMetaData(cqsi, metaData);
+ assertSame(metaData, cqsi.getMetaDataCache());
+ }
+
+ /**
+ * Builds a real ConnectionQueryServicesImpl so that getMetaDataCache() and
its private
+ * throwConnectionClosedIfNullMetaData() guard execute for real. A
CALLS_REAL_METHODS mock cannot
+ * be used here because Mockito does not route the internal call to the
private guard method.
+ */
+ private static ConnectionQueryServicesImpl newRealCqsi() {
+ QueryServices mockQueryServices = Mockito.mock(QueryServices.class);
+ ReadOnlyProps emptyProps = ReadOnlyProps.EMPTY_PROPS;
+ when(mockQueryServices.getProps()).thenReturn(emptyProps);
+ ConnectionInfo mockConnectionInfo = Mockito.mock(ConnectionInfo.class);
+ when(mockConnectionInfo.asProps()).thenReturn(emptyProps);
+ return new ConnectionQueryServicesImpl(mockQueryServices,
mockConnectionInfo, new Properties());
+ }
+
+ private static void setLatestMetaData(ConnectionQueryServicesImpl cqsi,
PMetaData value)
+ throws NoSuchFieldException, IllegalAccessException {
+ Field field =
ConnectionQueryServicesImpl.class.getDeclaredField("latestMetaData");
+ field.setAccessible(true);
+ field.set(cqsi, value);
+ }
}
