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.

Sign up for GitHub

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

Jump to bottom

io.camunda.zeebe.client.impl.oauth.OAuthCredentialsCache is not thread safe. #11885

Closed
aivinog1 opened this issue Mar 2, 2023 · 0 comments · Fixed by #11906
Closed

io.camunda.zeebe.client.impl.oauth.OAuthCredentialsCache is not thread safe. #11885

aivinog1 opened this issue Mar 2, 2023 · 0 comments · Fixed by #11906
Assignees
@remcowesterhoud @aivinog1
Labels
kind/bug Categorizes an issue or PR as a bug version:8.2.0 Marks an issue as being completely or in parts released in 8.2.0

Comments

@aivinog1
Copy link
Contributor

aivinog1 commented Mar 2, 2023

Describe the bug

After #11816 I never thought that the io.camunda.zeebe.client.impl.oauth.OAuthCredentialsProvider is not a thread-safe. So, when you start multiple JobWorker in parallel with OAuthCredentialsProvider you got a problem in this method: https://github.com/camunda/zeebe/blob/28fe054b79fcc7398a086a1576199efef01a122c/clients/java/src/main/java/io/camunda/zeebe/client/impl/oauth/OAuthCredentialsCache.java#L87-L111

So, in there is the first checking that the file exists, and then there is a creating one if none exists. If this part of the code is executed concurrently we get an error that we are trying to create a file that already exists.

To Reproduce

  1. Get a fresh main branch.
  2. Configure Zeebe Client with OAuthCredentialsProvider
  3. Start some requests in the parallel

Expected behavior

If requests with the Zeebe Client and OAuthCredentialsProvider runned at parallel there is no error about a file, that already created.

Log/Stacktrace

Full Stacktrace 

io.grpc.StatusException: CANCELLED
	at io.grpc.Status.asException(Status.java:554)
	at io.grpc.kotlin.ClientCalls$rpcImpl$1$1$1.onClose(ClientCalls.kt:296)
	at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:563)
	at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:70)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInternal(ClientCallImpl.java:744)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:723)
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: java.nio.file.FileAlreadyExistsException: /root/.camunda/credentials
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:94)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
	at java.base/java.nio.file.Files.newByteChannel(Files.java:375)
	at java.base/java.nio.file.Files.createFile(Files.java:652)
	at io.camunda.zeebe.client.impl.oauth.OAuthCredentialsCache.ensureCacheFileExists(OAuthCredentialsCache.java:110)
	at io.camunda.zeebe.client.impl.oauth.OAuthCredentialsCache.writeCache(OAuthCredentialsCache.java:69)
	at io.camunda.zeebe.client.impl.oauth.OAuthCredentialsProvider.refreshCredentials(OAuthCredentialsProvider.java:133)
	at io.camunda.zeebe.client.impl.oauth.OAuthCredentialsProvider.loadCredentials(OAuthCredentialsProvider.java:121)
	at io.camunda.zeebe.client.impl.oauth.OAuthCredentialsProvider.applyCredentials(OAuthCredentialsProvider.java:79)

Environment:

  • OS: MacOS, Linux
  • Zeebe Version: from main branch
  • Configuration: OAuthCredentialsProvider
@aivinog1 aivinog1 added the kind/bug Categorizes an issue or PR as a bug label Mar 2, 2023
@aivinog1 aivinog1 mentioned this issue Mar 3, 2023
Merged
14 tasks
ghost pushed a commit that referenced this issue Mar 16, 2023
@aivinog1
merge: #11906
d71e9be 
11906: fix(clients/java): make a `OAuthCredentialsProvider` class thread-safe r=remcowesterhoud a=aivinog1

## Description

<!-- Please explain the changes you made here. -->
I introduced some locking mechanisms in the OAuth2 integration in Zeebe Client Java to make it thread-safe.
I don't like the current solution but I don't have nice ideas on how to fix it in another way.

In the future, we could allow users to move [this call](https://github.com/camunda/zeebe/blob/main/clients/java/src/main/java/io/camunda/zeebe/client/impl/ZeebeCallCredentials.java#L49) inside their `CredentialsProvider`, because in the `io.grpc.CallCredentials` class [JavaDoc says](https://github.com/grpc/grpc-java/blob/5be17e8b22d1d3eb99ea92140af2297734100e63/api/src/main/java/io/grpc/CallCredentials.java#L39-L54): 
> If metadata is not immediately available, e.g., needs to be fetched from the network, the implementation may give the applier to an asynchronous task which will eventually call the applier. The RPC proceeds only after the applier is called.

And we could build a nice asynchronous pipeline without locking (in my mind it could be based on a queue or something like this: we just push some actions in the queue and a single consumer of this queue does all logic (thus these consumers calls could be lock-free and not thread-safe, but the whole construction will be thread safe)).

## Related issues

<!-- Which issues are closed by this PR or are related -->

closes #11885 



Co-authored-by: Alexey Vinogradov <vinogradov.a.i.93@gmail.com>
@ghost ghost closed this as completed in a91f20b Mar 16, 2023
@npepinpe npepinpe added the version:8.2.0 Marks an issue as being completely or in parts released in 8.2.0 label Apr 5, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 9, 2024
@RomanJRW
chore(deps): merge docker renovate config to single manager (#11885)
81a33d5
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@remcowesterhoud remcowesterhoud

@aivinog1 aivinog1

Labels
kind/bug Categorizes an issue or PR as a bug version:8.2.0 Marks an issue as being completely or in parts released in 8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(clients/java): make a OAuthCredentialsProvider class thread-safe aivinog1/zeebe
3 participants
@npepinpe @remcowesterhoud @aivinog1