riemenschneider opened a new issue, #18786:
URL: https://github.com/apache/iceberg/issues/18786
### Apache Iceberg version
1.11.0
### Query engine
Spark
### Please describe the bug 🐞
`VendedAzureSasCredentialPolicy` can throw a `NullPointerException` on the
first requests that go through a new `ADLSFileIO` when several threads send
them at the same moment.
The affected file is identical in 1.12.0 and on `main`.
```
java.lang.NullPointerException: Cannot invoke
"com.azure.core.http.policy.AzureSasCredentialPolicy.process(com.azure.core.http.HttpPipelineCallContext,
com.azure.core.http.HttpPipelineNextPolicy)" because
"this.azureSasCredentialPolicy" is null
at
org.apache.iceberg.azure.adlsv2.VendedAzureSasCredentialPolicy.lambda$process$0(VendedAzureSasCredentialPolicy.java:50)
at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:45)
at
reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:241)
```
**Cause**
In `maybeUpdateCredential`, the volatile `azureSasCredential` is published
before the plain field `azureSasCredentialPolicy` is assigned:
```java
private volatile AzureSasCredential azureSasCredential;
private AzureSasCredentialPolicy azureSasCredentialPolicy;
...
if (azureSasCredential == null) {
synchronized (this) {
if (azureSasCredential == null) {
this.azureSasCredential = new AzureSasCredential(sasToken);
this.azureSasCredentialPolicy = new
AzureSasCredentialPolicy(azureSasCredential, false);
return Mono.empty();
}
}
}
azureSasCredential.update(sasToken);
return Mono.empty();
```
A second thread can read `azureSasCredential` as non-null, skip the lock,
and reach `azureSasCredentialPolicy.process(...)` (line 50, and `processSync`
has the same pattern) while the first thread has not yet assigned the policy.
The policy field is also read without any happens-before edge to its write.
#13730 added the `volatile` and the double-checked lock on the credential
field, but not on the field that `process` dereferences.
`ADLSFileIO.buildClient` creates one policy per storage account per
`ADLSFileIO` instance, so any burst of parallel first requests on a new file IO
is exposed, for example parallel manifest reads right after a table load.
**Where I saw it**
Production, Azure, REST catalog with vended credentials: a Spark based
compaction job over roughly 3,300 tables per hourly run failed one or two
tables per run, at random, with this exception. The step failed about 0.4 s
after it started, and the next run compacted the same table. Log line before
the NPE, from one occurrence:
```
ERROR org.apache.iceberg.azure.adlsv2.ADLSInputStream - Failed to open input
stream for file <redacted>/metadata/<uuid>-m0.avro, range bytes=0-
```
**Reproducer**
A JUnit 5 test in the `org.apache.iceberg.azure.adlsv2` package drives the
real class through a real `HttpPipeline` with a stub `HttpClient`. In each
round it creates a fresh `VendedAzureSasCredentialPolicy` (the provider is a
subclass returning `Mono.just(token)`) and releases N threads together on a
spin barrier, each doing `pipeline.send(request).block()`.
Against the unmodified 1.11.0 jar:
| Threads | Rounds | Result |
|---|---|---|
| 8 | 200,000 | 31,427 NPEs in 19,286 rounds |
| 8 and 4 | 20,000 | the test fails in 8 of 8 runs (11 to 102 failing
rounds) |
The NPE message and the line 50 frame are the same as in production. With a
small round count the hit rate depends on JIT warm-up (one 2,000 round run at 8
threads hit nothing), so the test needs about 20,000 rounds (about 3 s).
**Proposed fix**
Make the field that `process` dereferences the volatile guard, and assign
the credential first:
```diff
- private volatile AzureSasCredential azureSasCredential;
- private AzureSasCredentialPolicy azureSasCredentialPolicy;
+ private AzureSasCredential azureSasCredential;
+ private volatile AzureSasCredentialPolicy azureSasCredentialPolicy;
...
- if (azureSasCredential == null) {
+ if (azureSasCredentialPolicy == null) {
synchronized (this) {
- if (azureSasCredential == null) {
+ if (azureSasCredentialPolicy == null) {
```
With this change the same test ran 1,000,000 rounds at 8 threads with no NPE.
**Limits of the evidence**
- The reproducer exercises the class in isolation. The provider returns
immediately and the threads are released together, so its hit rate is far above
what a real workload sees.
- The link to the production failures rests on the identical exception
message and frame. I read the stack trace for one occurrence only.
- I have not run the test in Iceberg's own Gradle build, and `spotlessApply`
and checkstyle have not run.
### Willingness to contribute
- [x] I can contribute a fix for this bug independently
---
**AI Disclosure**
- Model: Claude Sonnet 5.5
- Platform/Tool: Claude Code
- Human Oversight: partially reviewed. The analysis, reproducer and fix are
generated. I read the source of VendedAzureSasCredentialPolicy, confirmed the
write order, and ran the reproducer myself. I did not run it in Iceberg's
Gradle build.
- Prompt Summary: Analyze a production optimizer failure, find the cause in
the Iceberg Azure vended-credential code, write a reproducer and a candidate
fix. The concurrency reasoning and the reproducer are generated and need the
author's review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
