JanusGraph / JanusGraph/janusgraph

MixedIndexTypeWrapper.fields is a non-volatile lazily-initialised array, allowing unsafe publication

Open Beginner friendly
#4,927 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
5.8k
Forks
1.2k
Avg merge
13h 53m
Merged PRs (30d)
6

Description

- Version: `master` (`ac0eb23`); present in all released versions
- Storage Backend: any
- Mixed Index Backend: any (elasticsearch, solr, lucene)
- Expected Behavior: `MixedIndexTypeWrapper.getFieldKeys()` lazily initialises a shared array and should publish it safely, as the equivalent lazy field in its superclass does.
- Current Behavior: the field is **not** `volatile`, so another thread can observe a non-null array reference whose elements are still null.

### Details

`MixedIndexTypeWrapper` lazily builds and caches `fields`:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/types/indextype/MixedIndexTypeWrapper.java#L33-L66

```java
private ParameterIndexField[] fields = null; // not volatile

@Override
public ParameterIndexField[] getFieldKeys() {
ParameterIndexField[] result = fields;
if (result==null) {
...
result[pos++]=ParameterIndexField.of(...);
fields = result; // unsafe publication
}
return result;
}
```

The superclass does the same thing correctly — `IndexTypeWrapper.fieldMap` **is** declared `volatile`:

https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/types/indextype/IndexTypeWrapper.java#L39

```java
private volatile Map fieldMap = null;
```

`MixedIndexType` instances are shared across threads via the schema cache, so `getFieldKeys()` is called concurrently. Without `volatile` there is no happens-before edge between the array element writes and another thread's read of `fields`, so a second thread can see a non-null array containing null elements.

Downstream that surfaces as an NPE rather than anything diagnosable, because the only guard is an assertion that is disabled in production:

```java
// IndexRecordUtil.key2Field
public static String key2Field(ParameterIndexField field) {
assert field!=null;
return ParameterType.MAPPED_NAME.findParameter(field.getParameters(), ...); // NPE
}
```

Elasticsearch's provider dereferences `KeyInformation` without null checks in the same way (`ElasticSearchIndex.getAdditionDoc`, `getParameters`), so a torn read during a concurrent schema access can fail a mutation.

### Steps to Reproduce

Hard to reproduce deterministically, as with most publication races. It requires concurrent first-access to the same mixed index from multiple threads, ideally after `resetCache()` (a schema change) has nulled the field on a live graph.

### Suggested Fix

```java
private volatile ParameterIndexField[] fields = null;
```

Matching the superclass. Note that `getFieldKeys()` also returns the internal array directly rather than a copy, so callers can mutate shared schema state; worth considering separately.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in janusgraph-core/src/main/java/org/janusgraph/graphdb/types/indextype/MixedIndexTypeWrapper.java and inspect getFieldKeys(), then compare its fields declaration with IndexTypeWrapper.fieldMap. Make the cached array safely published and verify concurrent first access cannot observe null elements, including after resetCache() on a live graph.

Written by the indexing model from the issue text.

Assessment

Tech stack
elasticsearch, java
Domain
backend, database
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.