Skip to content

Commit 5509cf0

Browse files
jeet1995Copilot
jeet1995
and
Copilot
committed
Unify all accessor fields to static getter pattern, add enforcement test
- Convert all remaining instance accessor fields to static getter methods across 15 consuming classes (CosmosDiagnosticsContext was the last dangerous static final field running during <clinit>) - Add noStaticOrInstanceAccessorFieldsInConsumingClasses enforcement test that scans source files to prevent reintroduction of dangerous patterns - Consistent pattern: private static XxxAccessor xxx() { return getXxxAccessor(); } Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f793731 commit 5509cf0

16 files changed

+285
-127
lines changed

‎sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/ImplementationBridgeHelpersTest.java‎

Lines changed: 126 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void accessorInitialization() {
119119
* from 12 threads synchronized via a {@link CyclicBarrier}. In a fresh JVM, {@code <clinit>}
120120
* runs for the first time — the only way to exercise the real deadlock scenario. A 30-second
121121
* timeout detects the hang. Runs 5 invocations via TestNG ({@code invocationCount = 5}),
122-
* each forking 3 child JVMs — totaling 15 fresh JVMs × 12 concurrent threads = 180
122+
* each forking 1 child JVM — totaling 5 fresh JVMs × 12 concurrent threads = 60
123123
* {@code <clinit>} race attempts.
124124
*/
125125
@Test(groups = { "unit" }, invocationCount = 5)
@@ -446,4 +446,129 @@ public static void main(String[] args) throws Exception {
446446
}
447447
}
448448
}
449+
450+
/**
451+
* Enforces that no consuming class stores an accessor in a {@code static} field or
452+
* {@code final} instance field assigned at declaration. Such fields are initialized
453+
* during {@code <clinit>} (for static) or eagerly during construction (for instance
454+
* finals assigned inline), and can trigger {@code initializeAllAccessors()}, creating
455+
* circular class-initialization lock chains that deadlock under concurrent class
456+
* loading (JLS §12.4.2).
457+
* <p>
458+
* The approved pattern is a {@code private static} getter method:
459+
* <pre>{@code
460+
* private static XxxAccessor xxxAccessor() {
461+
* return ImplementationBridgeHelpers.XxxHelper.getXxxAccessor();
462+
* }
463+
* }</pre>
464+
* <p>
465+
* Uses reflection — immune to formatting, multiline declarations, and import aliases.
466+
*/
467+
@Test(groups = { "unit" })
468+
public void noStaticOrInstanceAccessorFieldsInConsumingClasses() throws Exception {
469+
// Step 1: Collect all Accessor interface types from ImplementationBridgeHelpers
470+
java.util.Set<Class<?>> accessorTypes = new java.util.HashSet<>();
471+
for (Class<?> inner : ImplementationBridgeHelpers.class.getDeclaredClasses()) {
472+
for (Class<?> nested : inner.getDeclaredClasses()) {
473+
if (nested.isInterface() && nested.getSimpleName().endsWith("Accessor")) {
474+
accessorTypes.add(nested);
475+
}
476+
}
477+
}
478+
479+
assertThat(accessorTypes)
480+
.as("Should find accessor interfaces in ImplementationBridgeHelpers")
481+
.isNotEmpty();
482+
483+
// Step 2: Classes that legitimately hold accessor AtomicReference fields
484+
java.util.Set<String> exemptClassNames = new java.util.HashSet<>(java.util.Arrays.asList(
485+
"com.azure.cosmos.implementation.ImplementationBridgeHelpers",
486+
"com.azure.cosmos.BridgeInternal",
487+
"com.azure.cosmos.models.ModelBridgeInternal",
488+
"com.azure.cosmos.util.UtilBridgeInternal"
489+
));
490+
491+
// Step 3: Force-load all cosmos classes so we can scan them
492+
// initializeAllAccessors() transitively loads the main classes
493+
ImplementationBridgeHelpers.initializeAllAccessors();
494+
495+
// Get all classes visible via the classloader that are in com.azure.cosmos
496+
// We use the source tree to enumerate class names, then load them
497+
java.nio.file.Path cosmosRoot = java.nio.file.Paths.get(
498+
System.getProperty("user.dir"))
499+
.getParent()
500+
.resolve("azure-cosmos")
501+
.resolve("src")
502+
.resolve("main")
503+
.resolve("java");
504+
505+
assertThat(java.nio.file.Files.exists(cosmosRoot))
506+
.as("azure-cosmos source root must exist at: " + cosmosRoot)
507+
.isTrue();
508+
509+
java.nio.file.Path javaRoot = cosmosRoot;
510+
List<String> violations = new ArrayList<>();
511+
512+
java.nio.file.Files.walk(cosmosRoot)
513+
.filter(p -> p.toString().endsWith(".java"))
514+
.forEach(p -> {
515+
// Convert file path to class name
516+
String relative = javaRoot.relativize(p).toString();
517+
String className = relative
518+
.replace(java.io.File.separatorChar, '.')
519+
.replaceAll("\\.java$", "");
520+
521+
if (exemptClassNames.contains(className)) {
522+
return;
523+
}
524+
525+
// Also skip inner classes of exempt classes
526+
for (String exempt : exemptClassNames) {
527+
if (className.startsWith(exempt + "$")) {
528+
return;
529+
}
530+
}
531+
532+
try {
533+
Class<?> cls = Class.forName(className, false,
534+
ImplementationBridgeHelpers.class.getClassLoader());
535+
536+
for (Field field : cls.getDeclaredFields()) {
537+
Class<?> fieldType = field.getType();
538+
539+
// Check if this field's type is one of the Accessor interfaces
540+
if (!accessorTypes.contains(fieldType)) {
541+
continue;
542+
}
543+
544+
int mods = field.getModifiers();
545+
boolean isStatic = java.lang.reflect.Modifier.isStatic(mods);
546+
boolean isFinal = java.lang.reflect.Modifier.isFinal(mods);
547+
548+
// Dangerous: any static accessor field (runs during <clinit>)
549+
// Also flag: final instance fields (assigned at declaration = eager init)
550+
if (isStatic) {
551+
violations.add(cls.getName() + "." + field.getName()
552+
+ " — static " + (isFinal ? "final " : "")
553+
+ fieldType.getSimpleName()
554+
+ " (runs during <clinit>, can deadlock)");
555+
} else if (isFinal) {
556+
violations.add(cls.getName() + "." + field.getName()
557+
+ " — final " + fieldType.getSimpleName()
558+
+ " (instance field assigned at declaration, "
559+
+ "prefer static getter method for consistency)");
560+
}
561+
}
562+
} catch (ClassNotFoundException | NoClassDefFoundError e) {
563+
// Skip classes that can't be loaded (e.g., optional dependencies)
564+
}
565+
});
566+
567+
assertThat(violations)
568+
.as("Found accessor fields that can trigger <clinit> deadlocks or are inconsistent "
569+
+ "with the approved static getter pattern.\n"
570+
+ "Use 'private static XxxAccessor xxx() { return getXxxAccessor(); }' instead.\n"
571+
+ "Violations:\n" + String.join("\n", violations))
572+
.isEmpty();
573+
}
449574
}

‎sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/CosmosAsyncClient.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,13 @@
7979
builder = CosmosClientBuilder.class,
8080
isAsync = true)
8181
public final class CosmosAsyncClient implements Closeable {
82-
private final ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.CosmosQueryRequestOptionsAccessor
83-
queryOptionsAccessor = ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.getCosmosQueryRequestOptionsAccessor();
84-
private final ImplementationBridgeHelpers.FeedResponseHelper.FeedResponseAccessor
85-
feedResponseAccessor = ImplementationBridgeHelpers.FeedResponseHelper.getFeedResponseAccessor();
82+
private static ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.CosmosQueryRequestOptionsAccessor queryOptionsAccessor() {
83+
return ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.getCosmosQueryRequestOptionsAccessor();
84+
}
85+
86+
private static ImplementationBridgeHelpers.FeedResponseHelper.FeedResponseAccessor feedResponseAccessor() {
87+
return ImplementationBridgeHelpers.FeedResponseHelper.getFeedResponseAccessor();
88+
}
8689

8790
private static final Logger logger = LoggerFactory.getLogger(CosmosAsyncClient.class);
8891

@@ -466,7 +469,7 @@ CosmosPagedFlux<CosmosDatabaseProperties> readAllDatabases(CosmosQueryRequestOpt
466469
null,
467470
ResourceType.Database,
468471
OperationType.ReadFeed,
469-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
472+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
470473
nonNullOptions,
471474
pagedFluxOptions
472475
);
@@ -475,7 +478,7 @@ CosmosPagedFlux<CosmosDatabaseProperties> readAllDatabases(CosmosQueryRequestOpt
475478

476479
return getDocClientWrapper().readDatabases(state)
477480
.map(response ->
478-
feedResponseAccessor.createFeedResponse(
481+
feedResponseAccessor().createFeedResponse(
479482
ModelBridgeInternal.getCosmosDatabasePropertiesFromV2Results(response.getResults()),
480483
response.getResponseHeaders(),
481484
response.getCosmosDiagnostics()));
@@ -652,15 +655,15 @@ private CosmosPagedFlux<CosmosDatabaseProperties> queryDatabasesInternal(
652655
null,
653656
ResourceType.Database,
654657
OperationType.Query,
655-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
658+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
656659
nonNullOptions,
657660
pagedFluxOptions
658661
);
659662

660663
pagedFluxOptions.setFeedOperationState(state);
661664

662665
return getDocClientWrapper().queryDatabases(querySpec, state)
663-
.map(response -> feedResponseAccessor.createFeedResponse(
666+
.map(response -> feedResponseAccessor().createFeedResponse(
664667
ModelBridgeInternal.getCosmosDatabasePropertiesFromV2Results(response.getResults()),
665668
response.getResponseHeaders(),
666669
response.getCosmosDiagnostics()));

‎sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/CosmosAsyncDatabase.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@
4444
* Perform read and delete databases, update database throughput, and perform operations on child resources
4545
*/
4646
public class CosmosAsyncDatabase {
47-
private final ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.CosmosQueryRequestOptionsAccessor
48-
queryOptionsAccessor = ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.getCosmosQueryRequestOptionsAccessor();
49-
private final ImplementationBridgeHelpers.FeedResponseHelper.FeedResponseAccessor
50-
feedResponseAccessor = ImplementationBridgeHelpers.FeedResponseHelper.getFeedResponseAccessor();
47+
private static ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.CosmosQueryRequestOptionsAccessor queryOptionsAccessor() {
48+
return ImplementationBridgeHelpers.CosmosQueryRequestOptionsHelper.getCosmosQueryRequestOptionsAccessor();
49+
}
50+
51+
private static ImplementationBridgeHelpers.FeedResponseHelper.FeedResponseAccessor feedResponseAccessor() {
52+
return ImplementationBridgeHelpers.FeedResponseHelper.getFeedResponseAccessor();
53+
}
5154

5255

5356
private final CosmosAsyncClient client;
@@ -648,15 +651,15 @@ public CosmosPagedFlux<CosmosContainerProperties> readAllContainers(CosmosQueryR
648651
null,
649652
ResourceType.DocumentCollection,
650653
OperationType.ReadFeed,
651-
queryOptionsAccessor.getQueryNameOrDefault(requestOptions, spanName),
654+
queryOptionsAccessor().getQueryNameOrDefault(requestOptions, spanName),
652655
requestOptions,
653656
pagedFluxOptions
654657
);
655658

656659
pagedFluxOptions.setFeedOperationState(state);
657660

658661
return getDocClientWrapper().readCollections(getLink(), state)
659-
.map(response -> feedResponseAccessor.createFeedResponse(
662+
.map(response -> feedResponseAccessor().createFeedResponse(
660663
ModelBridgeInternal.getCosmosContainerPropertiesFromV2Results(response.getResults()),
661664
response.getResponseHeaders(),
662665
response.getCosmosDiagnostics()));
@@ -956,15 +959,15 @@ CosmosPagedFlux<CosmosUserProperties> readAllUsers(CosmosQueryRequestOptions opt
956959
null,
957960
ResourceType.User,
958961
OperationType.ReadFeed,
959-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
962+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
960963
nonNullOptions,
961964
pagedFluxOptions
962965
);
963966

964967
pagedFluxOptions.setFeedOperationState(state);
965968

966969
return getDocClientWrapper().readUsers(getLink(), state)
967-
.map(response -> feedResponseAccessor.createFeedResponse(
970+
.map(response -> feedResponseAccessor().createFeedResponse(
968971
ModelBridgeInternal.getCosmosUserPropertiesFromV2Results(response.getResults()), response
969972
.getResponseHeaders(),
970973
response.getCosmosDiagnostics()));
@@ -1019,15 +1022,15 @@ public CosmosPagedFlux<CosmosClientEncryptionKeyProperties> readAllClientEncrypt
10191022
null,
10201023
ResourceType.ClientEncryptionKey,
10211024
OperationType.ReadFeed,
1022-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
1025+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
10231026
nonNullOptions,
10241027
pagedFluxOptions
10251028
);
10261029

10271030
pagedFluxOptions.setFeedOperationState(state);
10281031

10291032
return getDocClientWrapper().readClientEncryptionKeys(getLink(), state)
1030-
.map(response -> feedResponseAccessor.createFeedResponse(
1033+
.map(response -> feedResponseAccessor().createFeedResponse(
10311034
ModelBridgeInternal.getClientEncryptionKeyPropertiesList(response.getResults()), response
10321035
.getResponseHeaders(),
10331036
response.getCosmosDiagnostics()));
@@ -1120,7 +1123,7 @@ private CosmosPagedFlux<CosmosClientEncryptionKeyProperties> queryClientEncrypti
11201123
null,
11211124
ResourceType.ClientEncryptionKey,
11221125
OperationType.Query,
1123-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
1126+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
11241127
nonNullOptions,
11251128
pagedFluxOptions
11261129
);
@@ -1304,15 +1307,15 @@ private CosmosPagedFlux<CosmosContainerProperties> queryContainersInternal(SqlQu
13041307
null,
13051308
ResourceType.DocumentCollection,
13061309
OperationType.Query,
1307-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
1310+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
13081311
nonNullOptions,
13091312
pagedFluxOptions
13101313
);
13111314

13121315
pagedFluxOptions.setFeedOperationState(state);
13131316

13141317
return getDocClientWrapper().queryCollections(getLink(), querySpec, state)
1315-
.map(response -> feedResponseAccessor.createFeedResponse(
1318+
.map(response -> feedResponseAccessor().createFeedResponse(
13161319
ModelBridgeInternal.getCosmosContainerPropertiesFromV2Results(response.getResults()),
13171320
response.getResponseHeaders(),
13181321
response.getCosmosDiagnostics()));
@@ -1332,7 +1335,7 @@ private CosmosPagedFlux<CosmosUserProperties> queryUsersInternal(SqlQuerySpec qu
13321335
null,
13331336
ResourceType.User,
13341337
OperationType.Query,
1335-
queryOptionsAccessor.getQueryNameOrDefault(nonNullOptions, spanName),
1338+
queryOptionsAccessor().getQueryNameOrDefault(nonNullOptions, spanName),
13361339
nonNullOptions,
13371340
pagedFluxOptions
13381341
);

0 commit comments

Comments
 (0)