[
https://issues.apache.org/jira/browse/GEODE-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Leon Finker updated GEODE-10526:
--------------------------------
Description:
IndexTrackingQueryObserver.afterIndexLookup() throws a NullPointerException
when executing queries on partitioned regions during application startup. The
issue occurs because the method attempts to access a ThreadLocal
indexMap that was never initialized, causing the application to fail.
Root Cause
In org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.java, the
afterIndexLookup() method (line 107-110) retrieves a Map from a ThreadLocal
without checking for null:
{code:java}
@Override
public void afterIndexLookup(Collection results) {
if (results == null)
{ return; }
Map indexMap = (Map) indexInfo.get(); // Line 107 - Can be NULL
Index index = (Index) lastIndexUsed.get();
if (index != null) {
IndexInfo indexInfo = (IndexInfo) indexMap.get(...); // Line 110 -
NPE!
if (indexInfo != null)
{ indexInfo.getResults().put(index.getRegion().getFullPath(),
results.size()); }
}
// ...
}
{code}
The ThreadLocal indexMap is initialized in beforeIndexLookup() (lines 49-52),
but in certain scenarios involving partitioned region queries,
afterIndexLookup() can be called without beforeIndexLookup() having successfully
completed, leaving the ThreadLocal uninitialized.
Expected Behavior
- afterIndexLookup() should handle cases where beforeIndexLookup() did not
successfully initialize the ThreadLocal
- Query execution should complete successfully
- Application startup should not fail due to query observer issues
Actual Behavior
Application fails with:
{noformat}
java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
because "indexMap" is null
at
org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
Full stack trace:
java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
because "indexMap" is null
at
org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
at
org.apache.geode.cache.query.internal.CompiledComparison.singleBaseCollectionFilterEvaluate(CompiledComparison.java:502)
at
org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:181)
at
org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:227)
at
org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:532)
at
org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:53)
at
org.apache.geode.cache.query.internal.DefaultQuery.executeUsingContext(DefaultQuery.java:357)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeQueryOnBuckets(PRQueryProcessor.java:248)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeSequentially(PRQueryProcessor.java:214)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeQuery(PRQueryProcessor.java:123)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnLocalNode(PartitionedRegionQueryEvaluator.java:986)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnRemoteAndLocalNodes(PartitionedRegionQueryEvaluator.java:376)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.queryBuckets(PartitionedRegionQueryEvaluator.java:493)
at
org.apache.geode.internal.cache.PartitionedRegion.doExecuteQuery(PartitionedRegion.java:2134)
at
org.apache.geode.internal.cache.PartitionedRegion.executeQuery(PartitionedRegion.java:2061)
at
org.apache.geode.cache.query.internal.DefaultQuery.execute(DefaultQuery.java:241){noformat}
Suggested Fix
Add a null check in afterIndexLookup() method to handle uninitialized
ThreadLocal:
File:
geode-core/src/main/java/org/apache/geode/cache/query/internal/IndexTrackingQueryObserver.java
Change at lines 97-120:
{code:java}
@Override
public void afterIndexLookup(Collection results) {
if (results == null) { // according to javadocs in
QueryObserver, can be null if there // is an exception
return; }
// append the size of the lookup results (and bucket id if its an Index
on bucket)
// to IndexInfo results Map.
Map indexMap = (Map) indexInfo.get();
// Guard against uninitialized ThreadLocal in partitioned queries
if (indexMap == null) { // beforeIndexLookup was not called or
did not complete successfully // This can occur in partitioned region
query execution across buckets return; }
Index index = (Index) lastIndexUsed.get();
if (index != null) {
IndexInfo indexInfo = (IndexInfo) indexMap.get(getIndexName(index,
lastKeyUsed.get()));
if (indexInfo != null) {
indexInfo.getResults().put(index.getRegion().getFullPath(), results.size());
}
}
lastIndexUsed.set(null);
lastKeyUsed.set(null);
if (th != null)
{ th.hook(3); }
}
{code}
Rationale for Fix
1. Defensive Coding: Other methods like getUsedIndexes() (line 150-154)
already check for null before using the ThreadLocal
2. Consistency: The pattern if (results == null) return; is already used at
the method start
3. Minimal Impact: The fix is a simple guard clause that doesn't change
behavior when ThreadLocal is properly initialized
4. Safe Degradation: When the ThreadLocal is null, gracefully skip tracking
instead of crashing
Workaround
Until this bug is fixed, users can work around it by disabling query verbose
mode:
Option 1: JVM System Property
-Dgemfire.Query.VERBOSE=false
Setting gemfire.Query.VERBOSE=false completely disables
IndexTrackingQueryObserver creation and uses QueryObserverAdapter (with no-op
methods) instead, avoiding the bug entirely.
Additional Notes
1. Related Code Locations:
- IndexTrackingQueryObserver.java: Lines 42 (ThreadLocal declaration),
107-110 (bug location)
- CompiledComparison.java: Lines 383-387 (beforeIndexLookup call), 500-504
(afterIndexLookup call in finally)
- DefaultQuery.java: Line 781 (IndexTrackingQueryObserver instantiation)
2. Similar Patterns: The getUsedIndexes(String fullPath) method already
implements the null check pattern at line 209-212, which should be mirrored in
afterIndexLookup()
was:
IndexTrackingQueryObserver.afterIndexLookup() throws a NullPointerException
when executing queries on partitioned regions during application startup. The
issue occurs because the method attempts to access a ThreadLocal
indexMap that was never initialized, causing the application to fail.
Root Cause
In org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.java, the
afterIndexLookup() method (line 107-110) retrieves a Map from a ThreadLocal
without checking for null:
@Override
public void afterIndexLookup(Collection results) {
if (results == null) {
return;
}
Map indexMap = (Map) indexInfo.get(); // Line 107 - Can be NULL
Index index = (Index) lastIndexUsed.get();
if (index != null) {
IndexInfo indexInfo = (IndexInfo) indexMap.get(...); // Line 110 -
NPE!
if (indexInfo != null) {
indexInfo.getResults().put(index.getRegion().getFullPath(),
results.size());
}
}
// ...
}
The ThreadLocal indexMap is initialized in beforeIndexLookup() (lines 49-52),
but in certain scenarios involving partitioned region queries,
afterIndexLookup() can be called without beforeIndexLookup() having successfully
completed, leaving the ThreadLocal uninitialized.
Expected Behavior
- afterIndexLookup() should handle cases where beforeIndexLookup() did not
successfully initialize the ThreadLocal
- Query execution should complete successfully
- Application startup should not fail due to query observer issues
Actual Behavior
Application fails with:
{noformat}
java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
because "indexMap" is null
at
org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
Full stack trace:
java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
because "indexMap" is null
at
org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
at
org.apache.geode.cache.query.internal.CompiledComparison.singleBaseCollectionFilterEvaluate(CompiledComparison.java:502)
at
org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:181)
at
org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:227)
at
org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:532)
at
org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:53)
at
org.apache.geode.cache.query.internal.DefaultQuery.executeUsingContext(DefaultQuery.java:357)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeQueryOnBuckets(PRQueryProcessor.java:248)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeSequentially(PRQueryProcessor.java:214)
at
org.apache.geode.internal.cache.PRQueryProcessor.executeQuery(PRQueryProcessor.java:123)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnLocalNode(PartitionedRegionQueryEvaluator.java:986)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnRemoteAndLocalNodes(PartitionedRegionQueryEvaluator.java:376)
at
org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.queryBuckets(PartitionedRegionQueryEvaluator.java:493)
at
org.apache.geode.internal.cache.PartitionedRegion.doExecuteQuery(PartitionedRegion.java:2134)
at
org.apache.geode.internal.cache.PartitionedRegion.executeQuery(PartitionedRegion.java:2061)
at
org.apache.geode.cache.query.internal.DefaultQuery.execute(DefaultQuery.java:241){noformat}
Suggested Fix
Add a null check in afterIndexLookup() method to handle uninitialized
ThreadLocal:
File:
geode-core/src/main/java/org/apache/geode/cache/query/internal/IndexTrackingQueryObserver.java
Change at lines 97-120:
{code:java}
@Override
public void afterIndexLookup(Collection results) {
if (results == null) {
// according to javadocs in QueryObserver, can be null if there
// is an exception
return;
}
// append the size of the lookup results (and bucket id if its an Index
on bucket)
// to IndexInfo results Map.
Map indexMap = (Map) indexInfo.get();
// Guard against uninitialized ThreadLocal in partitioned queries
if (indexMap == null) {
// beforeIndexLookup was not called or did not complete successfully
// This can occur in partitioned region query execution across buckets
return;
}
Index index = (Index) lastIndexUsed.get();
if (index != null) {
IndexInfo indexInfo = (IndexInfo) indexMap.get(getIndexName(index,
lastKeyUsed.get()));
if (indexInfo != null) {
indexInfo.getResults().put(index.getRegion().getFullPath(),
results.size());
}
}
lastIndexUsed.set(null);
lastKeyUsed.set(null);
if (th != null) {
th.hook(3);
}
} {code}
Rationale for Fix
1. Defensive Coding: Other methods like getUsedIndexes() (line 150-154)
already check for null before using the ThreadLocal
2. Consistency: The pattern if (results == null) return; is already used at
the method start
3. Minimal Impact: The fix is a simple guard clause that doesn't change
behavior when ThreadLocal is properly initialized
4. Safe Degradation: When the ThreadLocal is null, gracefully skip tracking
instead of crashing
Workaround
Until this bug is fixed, users can work around it by disabling query verbose
mode:
Option 1: JVM System Property
-Dgemfire.Query.VERBOSE=false
Setting gemfire.Query.VERBOSE=false completely disables
IndexTrackingQueryObserver creation and uses QueryObserverAdapter (with no-op
methods) instead, avoiding the bug entirely.
Additional Notes
1. Related Code Locations:
- IndexTrackingQueryObserver.java: Lines 42 (ThreadLocal declaration),
107-110 (bug location)
- CompiledComparison.java: Lines 383-387 (beforeIndexLookup call), 500-504
(afterIndexLookup call in finally)
- DefaultQuery.java: Line 781 (IndexTrackingQueryObserver instantiation)
2. Similar Patterns: The getUsedIndexes(String fullPath) method already
implements the null check pattern at line 209-212, which should be mirrored in
afterIndexLookup()
> IndexTrackingQueryObserver.afterIndexLookup() throws NullPointerException
> when indexMap ThreadLocal is uninitialized in partitioned region queries
> --------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GEODE-10526
> URL: https://issues.apache.org/jira/browse/GEODE-10526
> Project: Geode
> Issue Type: Bug
> Components: querying
> Reporter: Leon Finker
> Priority: Major
>
> IndexTrackingQueryObserver.afterIndexLookup() throws a NullPointerException
> when executing queries on partitioned regions during application startup. The
> issue occurs because the method attempts to access a ThreadLocal
> indexMap that was never initialized, causing the application to fail.
> Root Cause
> In org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.java,
> the afterIndexLookup() method (line 107-110) retrieves a Map from a
> ThreadLocal without checking for null:
>
> {code:java}
> @Override
> public void afterIndexLookup(Collection results) {
> if (results == null)
> { return; }
> Map indexMap = (Map) indexInfo.get(); // Line 107 - Can be NULL
> Index index = (Index) lastIndexUsed.get();
> if (index != null) {
> IndexInfo indexInfo = (IndexInfo) indexMap.get(...); // Line 110 -
> NPE!
> if (indexInfo != null)
> { indexInfo.getResults().put(index.getRegion().getFullPath(),
> results.size()); }
> }
> // ...
> }
> {code}
> The ThreadLocal indexMap is initialized in beforeIndexLookup() (lines
> 49-52), but in certain scenarios involving partitioned region queries,
> afterIndexLookup() can be called without beforeIndexLookup() having
> successfully
> completed, leaving the ThreadLocal uninitialized.
>
> Expected Behavior
> - afterIndexLookup() should handle cases where beforeIndexLookup() did not
> successfully initialize the ThreadLocal
> - Query execution should complete successfully
> - Application startup should not fail due to query observer issues
> Actual Behavior
> Application fails with:
> {noformat}
> java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
> because "indexMap" is null
> at
> org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
> Full stack trace:
> java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)"
> because "indexMap" is null
> at
> org.apache.geode.cache.query.internal.IndexTrackingQueryObserver.afterIndexLookup(IndexTrackingQueryObserver.java:110)
> at
> org.apache.geode.cache.query.internal.CompiledComparison.singleBaseCollectionFilterEvaluate(CompiledComparison.java:502)
> at
> org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:181)
> at
> org.apache.geode.cache.query.internal.CompiledComparison.filterEvaluate(CompiledComparison.java:227)
> at
> org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:532)
> at
> org.apache.geode.cache.query.internal.CompiledSelect.evaluate(CompiledSelect.java:53)
> at
> org.apache.geode.cache.query.internal.DefaultQuery.executeUsingContext(DefaultQuery.java:357)
> at
> org.apache.geode.internal.cache.PRQueryProcessor.executeQueryOnBuckets(PRQueryProcessor.java:248)
> at
> org.apache.geode.internal.cache.PRQueryProcessor.executeSequentially(PRQueryProcessor.java:214)
> at
> org.apache.geode.internal.cache.PRQueryProcessor.executeQuery(PRQueryProcessor.java:123)
> at
> org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnLocalNode(PartitionedRegionQueryEvaluator.java:986)
> at
> org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.executeQueryOnRemoteAndLocalNodes(PartitionedRegionQueryEvaluator.java:376)
> at
> org.apache.geode.internal.cache.PartitionedRegionQueryEvaluator.queryBuckets(PartitionedRegionQueryEvaluator.java:493)
> at
> org.apache.geode.internal.cache.PartitionedRegion.doExecuteQuery(PartitionedRegion.java:2134)
> at
> org.apache.geode.internal.cache.PartitionedRegion.executeQuery(PartitionedRegion.java:2061)
> at
> org.apache.geode.cache.query.internal.DefaultQuery.execute(DefaultQuery.java:241){noformat}
> Suggested Fix
> Add a null check in afterIndexLookup() method to handle uninitialized
> ThreadLocal:
> File:
> geode-core/src/main/java/org/apache/geode/cache/query/internal/IndexTrackingQueryObserver.java
> Change at lines 97-120:
>
> {code:java}
> @Override
> public void afterIndexLookup(Collection results) {
> if (results == null) { // according to javadocs in
> QueryObserver, can be null if there // is an exception
> return; }
> // append the size of the lookup results (and bucket id if its an Index
> on bucket)
> // to IndexInfo results Map.
> Map indexMap = (Map) indexInfo.get();
> // Guard against uninitialized ThreadLocal in partitioned queries
> if (indexMap == null) { // beforeIndexLookup was not called
> or did not complete successfully // This can occur in partitioned
> region query execution across buckets return; }
> Index index = (Index) lastIndexUsed.get();
> if (index != null) {
> IndexInfo indexInfo = (IndexInfo) indexMap.get(getIndexName(index,
> lastKeyUsed.get()));
> if (indexInfo != null) {
> indexInfo.getResults().put(index.getRegion().getFullPath(), results.size());
> }
> }
> lastIndexUsed.set(null);
> lastKeyUsed.set(null);
> if (th != null)
> { th.hook(3); }
> }
> {code}
>
> Rationale for Fix
> 1. Defensive Coding: Other methods like getUsedIndexes() (line 150-154)
> already check for null before using the ThreadLocal
> 2. Consistency: The pattern if (results == null) return; is already used at
> the method start
> 3. Minimal Impact: The fix is a simple guard clause that doesn't change
> behavior when ThreadLocal is properly initialized
> 4. Safe Degradation: When the ThreadLocal is null, gracefully skip tracking
> instead of crashing
>
> Workaround
> Until this bug is fixed, users can work around it by disabling query
> verbose mode:
> Option 1: JVM System Property
> -Dgemfire.Query.VERBOSE=false
> Setting gemfire.Query.VERBOSE=false completely disables
> IndexTrackingQueryObserver creation and uses QueryObserverAdapter (with no-op
> methods) instead, avoiding the bug entirely.
>
> Additional Notes
> 1. Related Code Locations:
> - IndexTrackingQueryObserver.java: Lines 42 (ThreadLocal declaration),
> 107-110 (bug location)
> - CompiledComparison.java: Lines 383-387 (beforeIndexLookup call),
> 500-504 (afterIndexLookup call in finally)
> - DefaultQuery.java: Line 781 (IndexTrackingQueryObserver instantiation)
> 2. Similar Patterns: The getUsedIndexes(String fullPath) method already
> implements the null check pattern at line 209-212, which should be mirrored
> in afterIndexLookup()
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
