Thread Safety

[damioune123]

Hello,

Is the Java client supposed to be thread-safe?

If I look at the generated code, there is a section that is not threadsafe :

if(progressListener != null) {
            apiClient.getHttpClient().networkInterceptors().add(new com.squareup.okhttp.Interceptor() {
                @Override
                public com.squareup.okhttp.Response intercept(com.squareup.okhttp.Interceptor.Chain chain) throws IOException {
                    com.squareup.okhttp.Response originalResponse = chain.proceed(chain.request());
                    return originalResponse.newBuilder()
                    .body(new ProgressResponseBody(originalResponse.body(), progressListener))
                    .build();
                }
            });
        }

apiClient.getHttpClient() is a shared OkHttpCLient, and networkInterceptors is an ArrayList.

So multiple thread calling API methods could end up trying to concurrently modify this ArrayList.

[brendandburns]

Generally the client library doesn't add a progress listener, so in general, the client is thread-safe if you use the basic methods.

If you do add a progress listener, it's not thread-safe, both for the reason you mention and because the progress listeners will get attached to multiple requests. Indeed, the more I look at ProgressListener, the more I think it should be avoided...

Unfortunately, this is generated code, and I think it would be a pretty significant change to move remove this from the generated code.

The good news is this is a (mostly) internal method, and generally progressListener is null, so mostly you should be safe (isn't that always true...)

[damioune123]

Thank you for your quick response.
As I have to use async method, I have to define the API callback when calling my method ( public com.squareup.okhttp.Call connectGetNodeProxyWithPathAsync(String name, String path, String path2, final ApiCallback callback) throws ApiException, located in CoreV1API).

As you can see, a progress listener is always defined when the callback is defined

        if (callback != null) {
            progressListener = new ProgressResponseBody.ProgressListener() {
                @Override
                public void update(long bytesRead, long contentLength, boolean done) {
                    callback.onDownloadProgress(bytesRead, contentLength, done);
                }
            };

            progressRequestListener = new ProgressRequestBody.ProgressRequestListener() {
                @Override
                public void onRequestProgress(long bytesWritten, long contentLength, boolean done) {
                    callback.onUploadProgress(bytesWritten, contentLength, done);
                }
            };
        }

As you say if it's generated code, I'll create an issue at swagger github instead.

[mattnworb]

A similar issue we have been running into is that each async call causes an additional networkInterceptor to be added onto the OkHttpClient

if(progressListener != null) {
            apiClient.getHttpClient().networkInterceptors().add(new com.squareup.okhttp.Interceptor() {
            ...

the list of networkInterceptors grows unbounded in size, until eventually new async calls on e.g. CoreV1Api cause a StackOverflowError.

[mattnworb]

swagger-api/swagger-codegen#7876

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[mattnworb]

/remove-lifecycle rotten

[ZP-AlwaysWin]

/remove-lifecycle rotten

hi @mattnworb
the java client have four create namespace method
Async

public com.squareup.okhttp.Call createNamespaceAsync(.....)

the async method how to use？
If there are any examples, look at this method separately and don't know how to use it.

Best Wishes

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[mattnworb]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.