[
https://issues.apache.org/jira/browse/JCR-5095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18103025#comment-18103025
]
Sylwester Lachiewicz commented on JCR-5095:
-------------------------------------------
I looked into this again and can reproduce it locally, without Jenkins: running
{{TokenBasedLoginTest}} in a loop fails roughly 1 in 10 times (macOS, JDK 24).
One caveat for anyone trying: {{jackrabbit-core/target/repository}} has to be
removed between runs, otherwise you get unrelated "Failed to get Repository
instance" errors from a stale lock.
h3. Two symptoms, one race
The two error messages reported here are the same race detected at two
different points:
* {{javax.jcr.InvalidItemStateException: Could not find child <uuid> of node
<uuid>}} - thrown from {{ItemSaveOperation.validateTransientItems}} during the
{{session.save()}} in {{TokenProvider.createToken}}.
* {{failed to build path of <uuid>: <uuid> has no child entry for <uuid>}} -
thrown from {{HierarchyManagerImpl}} right after the save, while
{{TokenInfoImpl}} resolves {{tokenNode.getPath()}}.
Which one you get depends on where the concurrent commit lands, so a change
that only addresses one of them still leaves the test flaky.
h3. Why the node name is not the cause
The hypothesis in this issue was that the millisecond-precision timestamp
produces colliding node names under load, and [~reschke] tried adding a UUID to
the name the way Oak does, without success. That matches what I see: the name
is not what breaks.
Each login already runs on its own session - {{DefaultLoginModule.commit}}
explicitly creates one ("use a different session instance to create the token
node in order to prevent concurrent modifications with the shared system
session"). What the concurrent logins do share is the {{.tokens}} parent node.
When another session commits a child below that parent, this session's view of
the parent can lose the child node entry for its own pending token node. Unique
node names do not prevent that, which is why the UUID change had no effect.
h3. Suggested mitigation
{{getTokenParent}} already handles exactly this class of conflict for the
concurrent creation of the token store itself: it refreshes the session and
re-reads the parent. The creation of the token node *below* that parent had no
equivalent handling, so a single conflict failed the whole login.
PR: https://github.com/apache/jackrabbit/pull/368
It wraps the token node creation in a bounded retry (3 attempts) that discards
the doomed transient state with {{session.refresh(false)}} and re-reads the
token parent, catching both exception types above. The original exception is
rethrown once the attempts are exhausted, so behaviour on persistent failures
is unchanged.
Measured on {{TokenBasedLoginTest}}, clean repository per run: 1 failure in 12
runs before, 0 failures in 54 runs after. The full
{{org.apache.jackrabbit.core.security.authentication}} suite (114 tests) still
passes.
One point for reviewers: {{session.refresh(false)}} discards all transient
changes on the session. That is safe for {{DefaultLoginModule}}, which passes a
dedicated session, but {{TokenBasedAuthentication.createToken}} is public and
accepts a caller-supplied session. The existing {{getTokenParent}} conflict
path already calls {{session.refresh(false)}} on the same session, so this is
not a new hazard, but it now applies to a second code path.
Note this makes token creation tolerate the conflict; it does not remove the
underlying race in the transient state handling, so I would suggest keeping
this issue open even if the PR is merged.
> TokenBasedLoginTest is flaky
> ----------------------------
>
> Key: JCR-5095
> URL: https://issues.apache.org/jira/browse/JCR-5095
> Project: Jackrabbit Content Repository
> Issue Type: Bug
> Components: core
> Reporter: Konrad Windszus
> Priority: Major
>
> The Jenkins build is failing sometimes due to that
> ([https://ci-builds.apache.org/job/Jackrabbit/job/jackrabbit-trunk/).]
> The logs shows
> {code:java}
> [ERROR]
> org.apache.jackrabbit.core.security.authentication.token.TokenBasedLoginTest.testConcurrentLogin
> – Time elapsed: 2.463 s <<< FAILURE!junit.framework.AssertionFailedError:
> javax.jcr.LoginException: Failed to commit: failed to build path of
> b7a136a5-5247-460d-bd74-4e191872ca92: 18765f8a-895a-49c4-831b-dc5119faaacd
> has no child entry for b7a136a5-5247-460d-bd74-4e191872ca92
> at junit.framework.Assert.fail(Assert.java:57)
> at junit.framework.TestCase.fail(TestCase.java:223)
> at
> org.apache.jackrabbit.core.security.authentication.token.TokenBasedLoginTest.testConcurrentLogin(TokenBasedLoginTest.java:275)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at junit.framework.TestCase.runTest(TestCase.java:177)
> at junit.framework.TestCase.runBare(TestCase.java:142)
> at junit.framework.TestResult$1.protect(TestResult.java:122)
> at junit.framework.TestResult.runProtected(TestResult.java:142)
> at junit.framework.TestResult.run(TestResult.java:125)
> at org.apache.jackrabbit.test.JCRTestResult.run(JCRTestResult.java:75)
> at junit.framework.TestCase.run(TestCase.java:130)
> at org.apache.jackrabbit.test.AbstractJCRTest.run(AbstractJCRTest.java:476)
> at junit.framework.TestSuite.runTest(TestSuite.java:241)
> at junit.framework.TestSuite.run(TestSuite.java:236)
> at junit.framework.TestSuite.runTest(TestSuite.java:241)
> at junit.framework.TestSuite.run(TestSuite.java:236)
> at
> org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
> at
> org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
> at
> org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
> at
> org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
> at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
> at
> org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495){code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
