YTDB-165: Ytdb 165 add non throwing get to cas array clean rebased by logart · Pull Request #671 · JetBrains/youtrackdb

artem (logart) · 2026-02-19T15:49:38Z

Motivation:

While implementing #YTDB-165, I have a place where I do casArray.get(pageIndex) to check if the page is in the readCache already.

This operation fails for all new pages since they are not in the array yet.

Based on the logic of the method, I will then cache those pages and add them to the array.
Solution:

Add a flag (I know this is bad, but I feel like copying the method is even worse...) to the get method of CASObjectArray, so it will define either throw on out of bound (array behaviour) or return null (map behaviour).

An alternative solution would be to create an atomic operation. Something along the lines like getOrSet which will get the existing value or (if it is missing) set a computed one.

https://youtrack.jetbrains.com/issue/YTDB-495/Make-CASObjectArray-behave-like-a-map

gemini-code-assist · 2026-02-19T15:50:08Z

Summary of Changes

Hello artem (@logart), I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the CASObjectArray to offer more flexible retrieval methods, moving beyond strict array-like behavior to incorporate map-like characteristics. The primary goal is to enable checking for the presence of an element at a given index without necessarily throwing an exception if the index is out of bounds, which is particularly useful for optimizing cache interactions. This change introduces a configurable get operation and a dedicated getOrEmpty method, complemented by new constructors and a defaultPlaceholder for improved usability. Robust multi-threaded tests have been added to validate the concurrent integrity of these new functionalities.

Highlights

Enhanced CASObjectArray Get Behavior: The CASObjectArray class now includes an overloaded get method that accepts a throwOnOutOfBounds flag, allowing callers to control whether an ArrayIndexOutOfBoundsException is thrown or a default placeholder is returned when accessing an out-of-bounds index. A new getOrEmpty method was also added for convenience, which will return a default placeholder (or null) without throwing an exception.
Default Placeholder Support: A defaultPlaceholder field was introduced in CASObjectArray, along with new constructors, to allow specifying a default value to be returned by getOrEmpty when an element is not found. An overloaded set method was also added to utilize this default placeholder.
New Multi-threaded Tests: A new test file, CASObjectArrayMTTest.java, was added to provide comprehensive multi-threaded tests for add, set, and compareAndSet operations using the Fray testing framework, ensuring the concurrency safety of the enhanced CASObjectArray.
Build Configuration Update: The pom.xml file was updated to integrate the Fray Maven plugin and its runtime and JUnit dependencies, enabling the execution of the new multi-threaded tests.

Changelog

core/pom.xml
- Added fray-plugins-maven plugin to the build section.
- Included fray-runtime and fray-junit dependencies with test scope.
core/src/main/java/com/jetbrains/youtrackdb/internal/common/concur/collection/CASObjectArray.java
- Introduced defaultPlaceholder field.
- Added two new constructors: a no-argument constructor and one accepting a defaultPlaceholder.
- Added an overloaded set method that uses the defaultPlaceholder.
- Refactored the get(int index) method to call a new private get(int index, boolean throwOnOutOfBounds) method.
- Implemented getOrEmpty(int index) method to return the defaultPlaceholder (or null) for out-of-bounds indices.
core/src/test/java/com/jetbrains/youtrackdb/internal/common/concur/collection/CASObjectArrayMTTest.java
- Added new file CASObjectArrayMTTest.java.
- Implemented addShouldReturnCorrectIndex concurrency test.
- Implemented setShouldEventuallyUpdateArray concurrency test.
- Implemented compareAndSetShouldUpdateOnlyOnce concurrency test.
core/src/test/java/com/jetbrains/youtrackdb/internal/common/concur/collection/CASObjectArrayTest.java
- Added shouldThrowIfIndexIsOutOfBounds test case.
- Added shouldReturnNullIfElementDoesNotExistsLikeAMap test case.
- Added shouldReturnDefaultPlaceholderIfElementDoesNotExists test case.

Activity

The pull request was initiated by logart.
The motivation for this change stems from the need to check for page existence in a readCache (related to YTDB-165) without encountering ArrayIndexOutOfBoundsException for new pages.
The author considered and discussed alternative solutions, such as copying the method or implementing an atomic getOrSet operation.
The changes align with the broader goal of making CASObjectArray behave more like a map, as suggested in YTDB-495.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	Gemini (@gemini-code-assist)	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request modifies CASObjectArray to allow non-throwing get operations and introduces new test dependencies along with a multi-threaded test class. The primary modification involves adding a defaultPlaceholder field and a getOrEmpty method, along with an overloaded get method that accepts a throwOnOutOfBounds boolean, enhancing flexibility in element retrieval. However, the new get and set methods lack proper validation for negative indices, potentially leading to infinite loops and Denial of Service. Additionally, the default constructor introduces a regression where the set(int, T) method reliably crashes with a NullPointerException due to an uninitialized placeholder. Ensure explicit bounds checking is added and the default placeholder is safely handled.

gemini-code-assist · 2026-02-19T15:55:10Z

+  private T get(int index, boolean throwOnOutOfBounds) {
    final var size = this.size.get();

    if (size <= index) {
-      throw new ArrayIndexOutOfBoundsException("Requested " + index + ", size is " + size);
+      if (throwOnOutOfBounds) {
+        throw new ArrayIndexOutOfBoundsException("Requested " + index + ", size is " + size);
+      }
+      return defaultPlaceholder;
    }


The get method (and by extension getOrEmpty) does not properly validate that the index parameter is non-negative, potentially leading to a Denial of Service. The existing check if (size <= index) fails for negative indices because size is always non-negative. For index < -1, the code calculates a containerIndex of 31 and enters an infinite loop of Thread.yield() while waiting for a container that will never be created. A check for index < 0 should be added to ensure the method fails safely for all out-of-bounds indices.

This is fixed right in the beginning of the method.

gemini-code-assist · 2026-02-19T15:55:10Z

+  public T getOrEmpty(int index) {
+    return get(index, false);


The getOrEmpty method provides a convenient way to retrieve an element without throwing an ArrayIndexOutOfBoundsException, returning the defaultPlaceholder instead. This aligns with the motivation of the PR.

gemini-code-assist · 2026-02-19T15:55:10Z

  public T get(int index) {
+    return get(index, true);


The original get method is now a wrapper around the new private get method, ensuring backward compatibility while leveraging the new functionality.

gemini-code-assist · 2026-02-19T15:55:11Z

+  @Test
+  public void shouldThrowIfIndexIsOutOfBounds() {
+    final var array = new CASObjectArray<Integer>();
+
+    Assert.assertThrows(IndexOutOfBoundsException.class, () -> array.get(0));


This test case correctly verifies that the default get method still throws IndexOutOfBoundsException when the index is out of bounds, maintaining the original behavior.

gemini-code-assist · 2026-02-19T15:55:11Z

+  @Test
+  public void shouldReturnNullIfElementDoesNotExistsLikeAMap() {
+    final var array = new CASObjectArray<Integer>();
+
+    Assert.assertNull(array.getOrEmpty(0));


This test confirms that getOrEmpty returns null when no default placeholder is specified and the index is out of bounds, which is the expected map-like behavior.

gemini-code-assist · 2026-02-19T15:55:11Z

+  @Test
+  public void shouldReturnDefaultPlaceholderIfElementDoesNotExists() {
+    Integer defaultPlaceholder = (int) (Math.random() * Integer.MAX_VALUE);
+    final var array = new CASObjectArray<>(defaultPlaceholder);
+
+    Assert.assertSame(defaultPlaceholder, array.getOrEmpty(0));


This test ensures that getOrEmpty returns the specified defaultPlaceholder when the index is out of bounds, validating the new constructor and behavior.

github-actions · 2026-02-19T17:00:39Z

Qodana for JVM

46 new problems were found

Inspection name	Severity	Problems
`Local variable type can be omitted`	🔶 Warning	9
`Default annotation parameter value`	🔶 Warning	6
`Statement lambda can be replaced with expression lambda`	🔶 Warning	1
`Spelling`	◽️ Notice	27
`Grammar`	◽️ Notice	2
`Duplicated code fragment`	◽️ Notice	1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

Andrii Lomakin (andrii0lomakin) · 2026-02-24T07:43:33Z

+  public void set(int index, T value) {
+    if (defaultPlaceholder == null) {
+      throw new UnsupportedOperationException(
+          "set without placeholder only works with new CASObjectArray(defaultPlaceholder) constructed object."


String concatenation produces ...object.Please use...

Sorry, I don't get what should use instead.

Andrii Lomakin (andrii0lomakin) · 2026-02-24T07:44:02Z

    return container.compareAndSet(indexInsideContainer, oldValue, value);
  }

+  public T getOrEmpty(int index) {


getOrDefault is more appropriate

Andrii Lomakin (andrii0lomakin) · 2026-02-24T07:45:09Z

+
+  @Test
+  public void shouldReturnDefaultPlaceholderIfElementDoesNotExists() {
+    Integer defaultPlaceholder = (int) (Math.random() * Integer.MAX_VALUE);


That is non-reproducable test as seed is not printed.

Andrii Lomakin (andrii0lomakin) · 2026-02-24T07:46:33Z

+  private final T defaultPlaceholder;
+
+  public CASObjectArray() {
+    this(null);


seems like null can not be passed, does not it ?

It could, by default we are setting null for the new element(legacy behaviour which I want to keep).
That's why when you use set(index, value, placholder) method the placeholder is mandatory and non null.
With the new api I've added you either need to:

Use non default constructor with placeholder, this way you are free to use both set(index, value) and set(index, value, placeholder) methods, or

Use default constructor like it used to be, but then set(index, value) will throw an exception since null placeholder is indeed not allowed.

I've thought about this design for a while, and I don't really like it, but I don't know how to make this change backward compatible in other way.

Andrii Lomakin (andrii0lomakin) · 2026-02-24T07:47:39Z

        </configuration>
      </plugin>
+      <plugin>
+        <groupId>org.pastalab.fray.maven</groupId>


The core module's surefire is configured with surefire-junit47 , are fray tests really executed

github-actions · 2026-02-27T13:03:44Z

Coverage Gate Results

Thresholds: 85% line, 70% branch

Line Coverage: ✅ 89.5% (17/19 lines)

File	Coverage	Uncovered Lines
`core/src/main/java/com/jetbrains/youtrackdb/internal/common/concur/collection/CASObjectArray.java`	✅ 89.5% (17/19)	51-52

Branch Coverage: ✅ 83.3% (10/12 branches)

File	Coverage	Lines with Uncovered Branches
`core/src/main/java/com/jetbrains/youtrackdb/internal/common/concur/collection/CASObjectArray.java`	✅ 83.3% (10/12)	47, 106

github-actions · 2026-02-27T13:08:35Z

Mutation Testing Gate Results

Threshold: 85%

✅ No mutations found — mutation gate skipped.

Andrii Lomakin (andrii0lomakin) · 2026-03-03T06:53:18Z

artem (@logart) do we need this one ?

artem (logart) · 2026-03-03T09:17:29Z

Andrii Lomakin (@andrii0lomakin) not sure how to reply to specific comment, so adding a link: #671 (comment)

If you mean PR, yes, I am integrating your vmlense configuration from develop and checking if all my tests pass.

…tbrains.com/issue/YTDB-495/Make-CASObjectArray-behave-like-a-map for explanation.

Add default placeholder to simplify null/placeholder comparison.

Add a check on correct set usage with defaultPlaceholder value. Add more concurrent tests.

Fix issue found by review. Negative index hang check.

Replace fray with vmlens.

Revert junit 5 changes from pom.

Fix review comments. Rename getOrEmpty to getOrDefault. Don't use string concatenation. Print value of defaultPlaceholder if test fails.

Do not run vmlense and jacoco together. This cause false positive raises on the vmlense side.

Remove changes not related to my PR. Combine existing and my own tests in com.jetbrains.youtrackdb.internal.common.concur.collection.CASObjectArrayMTTest

github-actions · 2026-04-22T13:51:43Z

Test Count Gate Results

Tolerance: 5% drop allowed per module

Overall: ✅ 19959 tests (baseline: 19944, +15)

Module	Baseline	Current	Change	Status
`core`	9396	9411	+15	✅
`docker-tests`	1891	1891	+0	✅
`embedded`	1931	1931	+0	✅
`examples`	6	6	+0	✅
`gremlin-annotations`	30	30	+0	✅
`jmh-ldbc`	39	39	+0	✅
`server`	5504	5504	+0	✅
`tests`	1147	1147	+0	✅

github-actions Bot changed the title ~~Ytdb 165 add non throwing get to cas array clean rebased~~ YTDB-165: Ytdb 165 add non throwing get to cas array clean rebased Feb 19, 2026

gemini-code-assist Bot reviewed Feb 19, 2026

View reviewed changes

Andrii Lomakin (andrii0lomakin) self-requested a review February 24, 2026 07:50

Andrii Lomakin (andrii0lomakin) requested changes Feb 24, 2026

View reviewed changes

artem (logart) force-pushed the ytdb-165-add-non-throwing-get-to-cas-array-clean-rebased branch 3 times, most recently from 068af4e to df30def Compare February 27, 2026 10:19

artem (logart) force-pushed the ytdb-165-add-non-throwing-get-to-cas-array-clean-rebased branch from df30def to 4008d6d Compare March 3, 2026 09:14

artem (logart) force-pushed the ytdb-165-add-non-throwing-get-to-cas-array-clean-rebased branch from 4008d6d to 322da95 Compare March 3, 2026 09:35

artem.loginov added 10 commits April 22, 2026 12:01

Add a flag to not throw OutOfBoundsException. See https://youtrack.je…

a507566

…tbrains.com/issue/YTDB-495/Make-CASObjectArray-behave-like-a-map for explanation.

#YTDB-165

d5ab232

Add default placeholder to simplify null/placeholder comparison.

#YTDB-165

129c25d

Add a check on correct set usage with defaultPlaceholder value. Add more concurrent tests.

#YTDB-165

b6e1c4d

Fix issue found by review. Negative index hang check.

#YTDB-165

005c669

Replace fray with vmlens.

#YTDB-165

18a097c

Revert junit 5 changes from pom.

#YTDB-165

7efb9d9

Fix review comments. Rename getOrEmpty to getOrDefault. Don't use string concatenation. Print value of defaultPlaceholder if test fails.

YTDB-165

54e4dd5

Do not run vmlense and jacoco together. This cause false positive raises on the vmlense side.

Remove special vmlense config, it's hadled with argLine.

5d491e6

Disable vmlense when ekstazi is active.

fa5faad

artem (logart) force-pushed the ytdb-165-add-non-throwing-get-to-cas-array-clean-rebased branch from e35566c to fa5faad Compare April 22, 2026 10:14

#YTDB-165

e005ff6

Remove changes not related to my PR. Combine existing and my own tests in com.jetbrains.youtrackdb.internal.common.concur.collection.CASObjectArrayMTTest

artem (logart) requested a review from Andrii Lomakin (andrii0lomakin) April 22, 2026 13:40

Conversation

artem (logart) commented Feb 19, 2026

Uh oh!

gemini-code-assist Bot commented Feb 19, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

artem (logart) Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Qodana for JVM

Uh oh!

Andrii Lomakin (andrii0lomakin) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

artem (logart) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Andrii Lomakin (andrii0lomakin) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Andrii Lomakin (andrii0lomakin) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Andrii Lomakin (andrii0lomakin) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

artem (logart) Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrii Lomakin (andrii0lomakin) Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Gate Results

Line Coverage: ✅ 89.5% (17/19 lines)

Branch Coverage: ✅ 83.3% (10/12 branches)

Uh oh!

github-actions Bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mutation Testing Gate Results

Uh oh!

Andrii Lomakin (andrii0lomakin) commented Mar 3, 2026

Uh oh!

artem (logart) commented Mar 3, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Test Count Gate Results

Overall: ✅ 19959 tests (baseline: 19944, +15)

Uh oh!

Reviewers

Assignees

github-actions Bot commented Feb 19, 2026 •

edited

Loading

artem (logart) Feb 24, 2026 •

edited

Loading

github-actions Bot commented Feb 27, 2026 •

edited

Loading

github-actions Bot commented Feb 27, 2026 •

edited

Loading