Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPE in software.amazon.awssdk.utils.async.InputStreamSubscriber #4081

Closed
AlessandroPatti opened this issue Jun 8, 2023 · 4 comments · Fixed by #4460
Closed

NPE in software.amazon.awssdk.utils.async.InputStreamSubscriber #4081

AlessandroPatti opened this issue Jun 8, 2023 · 4 comments · Fixed by #4460
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@AlessandroPatti
Copy link

Describe the bug

Occasionally seeing the following when downloading from S3

java.lang.NullPointerException
	at software.amazon.awssdk.utils.async.InputStreamSubscriber.close(InputStreamSubscriber.java:123)
	at software.amazon.awssdk.core.io.SdkFilterInputStream.close(SdkFilterInputStream.java:83)

Expected Behavior

InputStreamSubscriber.close should not raise a NPE.

Current Behavior

java.lang.NullPointerException is being thrown occasionally when closing inout stream.

Reproduction Steps

It is hard to repro, as this only happens occasionally.

Possible Solution

I think there might be a race condition causing a variable to be used before it is initialised. Adding a snapshot of the relevant code from InputStreamSubscriber below:

 38 public final class InputStreamSubscriber extends InputStream implements Subscriber<ByteBuffer>, SdkAutoCloseable {
...
 44     private final AtomicReference<State> inputStreamState = new AtomicReference<>(State.UNINITIALIZED);
...
 48     private Subscription subscription;
...
 57     public void onSubscribe(Subscription s) {
 58         if (!inputStreamState.compareAndSet(State.UNINITIALIZED, State.READABLE)) {
 59             close();
 60             return;
 61         }
 62
 63         this.subscription = new CancelWatcher(s);
 64         delegate.onSubscribe(subscription);
 65     }
...
118     public void close() {
119         if (inputStreamState.compareAndSet(State.UNINITIALIZED, State.CLOSED)) {
120             delegate.onSubscribe(new NoOpSubscription());
121             delegate.onError(new CancellationException());
122         } else if (inputStreamState.compareAndSet(State.READABLE, State.CLOSED)) {
123             subscription.cancel();
124             onError(new CancellationException());
125         }
126     }

Considering the case where two threads access the class, one invoking the onSubscribe and the other close, it seems a race condition might cause the NPE when

  • Thread1 invokes onSubscribe and executes the first statement (line 58), setting the state to READABLE
  • Thread2 invokes close after Thread1 executed line 58, but before it gets to execute line 63
    • Since the state is now READABLE, Thread2 attempts to call subscribe.cancel() on an unitialised subcriber.

Additional Information/Context

No response

AWS Java SDK version used

2.20.59

JDK version used

11

Operating System and version

MacOS 13

@AlessandroPatti AlessandroPatti added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 8, 2023
@debora-ito
Copy link
Member

@AlessandroPatti I see you said the error is hard to repro, but can you provide a sample code showing how you're using the InputStreamSubscriber?

@debora-ito debora-ito added needs-review This issue or PR needs review from the team. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 6, 2023
@AlessandroPatti
Copy link
Author

@debora-ito I am following the coursier documentation to download from a maven repository stored in a s3 bucket. The code looks very similar to this example, except it uses the sdk v2 and the S3AsyncClient is wrapped in a S3TransferManager

@debora-ito debora-ito added the p2 This is a standard priority issue label Jul 11, 2023
@debora-ito
Copy link
Member

The possible race condition makes sense. We added a task to our backlog to investigate this.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
2 participants