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

[batrived]

• 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:

janusgraph/janusgraph-core/src/main/java/org/janusgraph/graphdb/types/indextype/MixedIndexTypeWrapper.java

Lines 33 to 66 in ac0eb23

	private ParameterIndexField[] fields = null;
	public MixedIndexTypeWrapper(SchemaSource base) {
	super(base);
	}
	@Override
	public boolean isCompositeIndex() {
	return false;
	}
	@Override
	public boolean isMixedIndex() {
	return true;
	}
	@Override
	public ParameterIndexField[] getFieldKeys() {
	ParameterIndexField[] result = fields;
	if (result==null) {
	List<SchemaSource.Entry> entries = base.getRelated(TypeDefinitionCategory.INDEX_FIELD,Direction.OUT);
	int numFields = entries.size();
	result = new ParameterIndexField[numFields];
	int pos = 0;
	for (SchemaSource.Entry entry : entries) {
	assert entry.getSchemaType() instanceof PropertyKey;
	assert entry.getModifier() instanceof Parameter[];
	result[pos++]=ParameterIndexField.of((PropertyKey)entry.getSchemaType(),(Parameter[])entry.getModifier());
	}
	fields = result;
	}
	assert result!=null;
	return result;
	}

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:

janusgraph/janusgraph-core/src/main/java/org/janusgraph/graphdb/types/indextype/IndexTypeWrapper.java

Line 39 in ac0eb23

private volatile Map<PropertyKey,IndexField> fieldMap = null;

private volatile Map<PropertyKey,IndexField> 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:

// 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

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.