Skip to content

test(framework): await reward calculation before mutating store - #7025

Merged
kuny0707 merged 3 commits into
tronprotocol:release_v4.8.3from
317787106:fix/compute-reward-test-race
Oct 13, 2026
Merged

kuny0707 merged 3 commits into
tronprotocol:release_v4.8.3from
317787106:fix/compute-reward-test-race

Conversation

@317787106

@317787106 317787106 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes a race in ComputeRewardTest.query() that can terminate the Gradle test worker while the test closes its database. After verifying the initial reward query, the test shuts down the reward executor and asserts that it terminates within 10 seconds before modifying or closing RewardViStore.

The existing reward-value and expected-TronError assertions remain in place. If the background task does not finish within the bound, the test fails before performing those store operations.

Why are these changes required?

RewardViCalService.clearUp() releases its completion latch before calcMerkleRoot() finishes. Consequently, queryReward() can return while the background task still uses the store. The test then manually invokes calculation paths and closes the store to exercise error handling, without waiting for that background task to finish.

In the captured Debian/JDK 8 failure, the background calculation accessed the closed LevelDB, raised REWARD_VI_CALCULATOR, and triggered System.exit(1). The test's synchronous assertThrows could not intercept an error from another thread, so the test could report success while its worker exited abnormally.

A separate run of the same test recorded overlapping Merkle-root calculations, store closure, pthread lock: Invalid argument, and worker exit 134. This is consistent with the same store-lifetime race, although no native stack trace was available to identify the exact failing call.

shutdown() alone does not wait for an in-flight task. The termination assertion establishes the required ordering between background calculation and the test's store operations.

This PR has been tested by:

  • Three independent local executions of ComputeRewardTest.query() on JDK 17 / ARM64, with test-result reuse, build cache, and automatic test retries disabled; all passed.
  • checkstyleMain and checkstyleTest: passed. Semgrep on the changed test: zero findings.
  • The fix was also validated in the now-closed diagnostic PR ci(framework): bound linux workers and retain test diagnostics #7024: a fresh focused test and Checkstyle passed after integration, and the CI build matrix passed on macOS/JDK 17, Ubuntu ARM64/JDK 17, Rocky Linux/JDK 8, and Debian/JDK 8. That diagnostic workflow forced task execution on Ubuntu and Rocky Linux with two Gradle workers.

Follow up

Monitor the regular CI configuration for recurrence of test-worker exits, particularly native exit 134.

Extra details

Targets release_v4.8.3. This is a Java 8-compatible, test-only change. The existing 60-second test timeout is preserved; the diagnostic workflow and JVM configuration changes from #7024 are not included.

@github-actions
github-actions Bot requested a review from xxo1shine October 13, 2026 02:36
Comment thread framework/src/test/java/org/tron/core/services/ComputeRewardTest.java Outdated
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.3 milestone Oct 13, 2026
@kuny0707
kuny0707 merged commit e1ffb1c into tronprotocol:release_v4.8.3 Oct 13, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants