Skip to content

Commit 118dcca

Browse files
AXIS2-5904 Fix policy cache race condition (millisecond granularity)
AxisBindingMessage.getEffectivePolicy() used Date.after() to detect policy changes. Two policy updates within the same millisecond produced identical timestamps, causing isPolicyUpdated() to return false and a stale (potentially null) cached policy to be returned. This manifested as intermittent "Rampart policy configuration missing" errors on consecutive secured SOAP calls. Replace Date-based timestamp comparison with a monotonic AtomicLong counter in PolicySubject. Every mutation (attach, detach, update, clear) increments the global counter. AxisBindingMessage.isPolicyUpdated() now compares version numbers instead of timestamps — no granularity issues, no race. The deprecated Date-based getLastUpdatedTime()/setLastUpdatedTime() API is preserved for backward compatibility. Includes regression test (PolicyCacheRaceTest, 5 tests) that verifies version monotonicity and rapid-fire policy updates are all detected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1832db2 commit 118dcca

3 files changed

Lines changed: 163 additions & 35 deletions

File tree

modules/kernel/src/org/apache/axis2/description/AxisBindingMessage.java

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public class AxisBindingMessage extends AxisDescription {
5353

5454
private volatile Policy effectivePolicy = null;
5555
private volatile Date lastPolicyCalculatedTime = null;
56+
private volatile long lastPolicyCalculatedVersion = -1;
5657

5758
public boolean isFault() {
5859
return fault;
@@ -217,11 +218,12 @@ public AxisBindingOperation getAxisBindingOperation() {
217218
}
218219

219220
public Policy getEffectivePolicy() {
220-
if (lastPolicyCalculatedTime == null || isPolicyUpdated()) {
221+
if (isPolicyUpdated()) {
221222
synchronized (this) {
222-
if (lastPolicyCalculatedTime == null || isPolicyUpdated()) {
223+
if (isPolicyUpdated()) {
223224
effectivePolicy = calculateEffectivePolicy();
224225
lastPolicyCalculatedTime = new Date();
226+
lastPolicyCalculatedVersion = getMaxPolicyVersion();
225227
}
226228
}
227229
}
@@ -293,64 +295,56 @@ public Policy calculateEffectivePolicy() {
293295
}
294296

295297
private boolean isPolicyUpdated() {
296-
if (getPolicySubject().getLastUpdatedTime().after(
297-
lastPolicyCalculatedTime)) {
298-
return true;
299-
}
298+
return getMaxPolicyVersion() > lastPolicyCalculatedVersion;
299+
}
300+
301+
/**
302+
* Returns the maximum policy version across the entire description
303+
* hierarchy. Uses monotonic counter instead of Date to avoid the
304+
* millisecond-granularity race condition in AXIS2-5904.
305+
*/
306+
private long getMaxPolicyVersion() {
307+
long max = getPolicySubject().getVersion();
300308
// AxisBindingOperation
301309
AxisBindingOperation axisBindingOperation = getAxisBindingOperation();
302-
if (axisBindingOperation != null
303-
&& axisBindingOperation.getPolicySubject().getLastUpdatedTime()
304-
.after(lastPolicyCalculatedTime)) {
305-
return true;
310+
if (axisBindingOperation != null) {
311+
max = Math.max(max, axisBindingOperation.getPolicySubject().getVersion());
306312
}
307313
// AxisBinding
308314
AxisBinding axisBinding = (axisBindingOperation == null) ? null
309315
: axisBindingOperation.getAxisBinding();
310-
if (axisBinding != null
311-
&& axisBinding.getPolicySubject().getLastUpdatedTime().after(
312-
lastPolicyCalculatedTime)) {
313-
return true;
316+
if (axisBinding != null) {
317+
max = Math.max(max, axisBinding.getPolicySubject().getVersion());
314318
}
315319
// AxisEndpoint
316320
AxisEndpoint axisEndpoint = (axisBinding == null) ? null : axisBinding
317321
.getAxisEndpoint();
318-
if (axisEndpoint != null
319-
&& axisEndpoint.getPolicySubject().getLastUpdatedTime().after(
320-
lastPolicyCalculatedTime)) {
321-
return true;
322+
if (axisEndpoint != null) {
323+
max = Math.max(max, axisEndpoint.getPolicySubject().getVersion());
322324
}
323325
// AxisMessage
324-
if (axisMessage != null
325-
&& axisMessage.getPolicySubject().getLastUpdatedTime().after(
326-
lastPolicyCalculatedTime)) {
327-
return true;
326+
if (axisMessage != null) {
327+
max = Math.max(max, axisMessage.getPolicySubject().getVersion());
328328
}
329329
// AxisOperation
330330
AxisOperation axisOperation = (axisMessage == null) ? null
331331
: axisMessage.getAxisOperation();
332-
if (axisOperation != null
333-
&& axisOperation.getPolicySubject().getLastUpdatedTime().after(
334-
lastPolicyCalculatedTime)) {
335-
return true;
332+
if (axisOperation != null) {
333+
max = Math.max(max, axisOperation.getPolicySubject().getVersion());
336334
}
337335
// AxisService
338336
AxisService axisService = (axisOperation == null) ? null
339337
: axisOperation.getAxisService();
340-
if (axisService != null
341-
&& axisService.getPolicySubject().getLastUpdatedTime().after(
342-
lastPolicyCalculatedTime)) {
343-
return true;
338+
if (axisService != null) {
339+
max = Math.max(max, axisService.getPolicySubject().getVersion());
344340
}
345341
// AxisConfiguration
346342
AxisConfiguration axisConfiguration = (axisService == null) ? null
347343
: axisService.getAxisConfiguration();
348-
if (axisConfiguration != null
349-
&& axisConfiguration.getPolicySubject().getLastUpdatedTime()
350-
.after(lastPolicyCalculatedTime)) {
351-
return true;
344+
if (axisConfiguration != null) {
345+
max = Math.max(max, axisConfiguration.getPolicySubject().getVersion());
352346
}
353-
return false;
347+
return max;
354348
}
355349

356350
@Override

modules/kernel/src/org/apache/axis2/description/PolicySubject.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@
2929
import java.util.Iterator;
3030
import java.util.List;
3131
import java.util.concurrent.ConcurrentHashMap;
32+
import java.util.concurrent.atomic.AtomicLong;
3233

3334
public class PolicySubject {
35+
private static final AtomicLong VERSION_COUNTER = new AtomicLong();
36+
private volatile long version = VERSION_COUNTER.incrementAndGet();
37+
@Deprecated
3438
private Date lastUpdatedTime = new Date();
3539

3640
private ConcurrentHashMap<String, PolicyComponent> attachedPolicyComponents = new ConcurrentHashMap<String, PolicyComponent>();
@@ -113,5 +117,10 @@ public Date getLastUpdatedTime() {
113117

114118
public void setLastUpdatedTime(Date lastUpdatedTime) {
115119
this.lastUpdatedTime = lastUpdatedTime;
120+
this.version = VERSION_COUNTER.incrementAndGet();
121+
}
122+
123+
public long getVersion() {
124+
return version;
116125
}
117126
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.axis2.description;
21+
22+
import org.apache.neethi.Policy;
23+
import org.junit.Test;
24+
25+
import static org.junit.Assert.*;
26+
27+
/**
28+
* Regression test for AXIS2-5904: Intermittent "Rampart policy configuration
29+
* missing" error caused by millisecond-granularity race in
30+
* {@link AxisBindingMessage#getEffectivePolicy()}.
31+
*
32+
* The old implementation used {@code Date.after()} to detect policy changes.
33+
* Two policy updates within the same millisecond would produce identical
34+
* timestamps, causing {@code isPolicyUpdated()} to return false and the
35+
* cached (potentially stale/null) policy to be returned. The fix replaces
36+
* Date comparison with a monotonic AtomicLong counter in PolicySubject.
37+
*/
38+
public class PolicyCacheRaceTest {
39+
40+
@Test
41+
public void testPolicyVersionIncrements() {
42+
PolicySubject subject = new PolicySubject();
43+
long v1 = subject.getVersion();
44+
45+
Policy policy = new Policy();
46+
policy.setId("test-policy-1");
47+
subject.attachPolicy(policy);
48+
long v2 = subject.getVersion();
49+
50+
assertTrue("Version must increment after attachPolicy", v2 > v1);
51+
52+
Policy policy2 = new Policy();
53+
policy2.setId("test-policy-2");
54+
subject.attachPolicy(policy2);
55+
long v3 = subject.getVersion();
56+
57+
assertTrue("Version must increment on each mutation", v3 > v2);
58+
}
59+
60+
@Test
61+
public void testPolicyVersionIncrementsOnDetach() {
62+
PolicySubject subject = new PolicySubject();
63+
Policy policy = new Policy();
64+
policy.setId("detach-test");
65+
subject.attachPolicy(policy);
66+
long vBefore = subject.getVersion();
67+
68+
subject.detachPolicyComponent("detach-test");
69+
long vAfter = subject.getVersion();
70+
71+
assertTrue("Version must increment after detach", vAfter > vBefore);
72+
}
73+
74+
@Test
75+
public void testPolicyVersionIncrementsOnClear() {
76+
PolicySubject subject = new PolicySubject();
77+
long vBefore = subject.getVersion();
78+
79+
subject.clear();
80+
long vAfter = subject.getVersion();
81+
82+
assertTrue("Version must increment after clear", vAfter > vBefore);
83+
}
84+
85+
@Test
86+
public void testEffectivePolicyDetectsUpdate() {
87+
// Build a minimal AxisBindingMessage hierarchy
88+
AxisBindingMessage bindingMessage = new AxisBindingMessage();
89+
90+
// First call — should compute and cache (returns null since no
91+
// policies are attached, but the caching mechanism still runs)
92+
Policy first = bindingMessage.getEffectivePolicy();
93+
94+
// Attach a policy to the binding message's own PolicySubject
95+
Policy policy = new Policy();
96+
policy.setId("axis2-5904-test");
97+
bindingMessage.getPolicySubject().attachPolicy(policy);
98+
99+
// Second call — must detect the version change and recompute.
100+
// Before the fix, if both calls happened within the same
101+
// millisecond, isPolicyUpdated() returned false and the stale
102+
// cached value was returned.
103+
Policy second = bindingMessage.getEffectivePolicy();
104+
105+
// The recomputed policy should include the attached policy
106+
assertNotNull("Effective policy must not be null after attaching a policy", second);
107+
}
108+
109+
@Test
110+
public void testRapidFirePolicyUpdatesAllDetected() {
111+
// Simulate the AXIS2-5904 scenario: multiple policy mutations
112+
// within the same millisecond must all be detected
113+
AxisBindingMessage bindingMessage = new AxisBindingMessage();
114+
115+
for (int i = 0; i < 100; i++) {
116+
Policy policy = new Policy();
117+
policy.setId("rapid-fire-" + i);
118+
bindingMessage.getPolicySubject().attachPolicy(policy);
119+
120+
// Every call must see the latest policy, not a stale cache
121+
Policy effective = bindingMessage.getEffectivePolicy();
122+
assertNotNull("Effective policy must not be null on iteration " + i, effective);
123+
}
124+
}
125+
}

0 commit comments

Comments
 (0)