Skip to content

Commit e46f8fb

Browse files
anicho123anicho123
authored
Merge commit from fork
PSIRT140 Fix
2 parents 68e7577 + fad089d commit e46f8fb

File tree

5 files changed

+570
-24
lines changed

5 files changed

+570
-24
lines changed

‎README.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* [Need help?](#need-help)
1111
* [Getting started](#getting-started)
1212
* [Usage guide](#usage-guide)
13+
* [Thread safety considerations](#thread-safety-considerations)
1314
* [Spring Support](#spring-support)
1415
* [Configuration reference](#configuration-reference)
1516
* [Building the SDK](#building-the-sdk)
@@ -177,6 +178,10 @@ These examples will help you understand how to use this library. You can also br
177178
Once you initialize a `ApiClient` instance, you can pass this instance to the constructor of any API area clients (such as `UserApi`, `GroupApi`, `ApplicationApi` etc.).
178179
You can start using these clients to call management APIs relevant to the chosen API area.
179180

181+
### Thread safety considerations
182+
183+
**Important:** The SDK stores pagination metadata keyed by thread ID. Sharing a single `ApiClient` across multiple threads may leak some internal state and grow memory usage inside large thread pools. When the SDK detects concurrent access it emits a warning (once per `ApiClient`) through the installed SLF4J logger.
184+
180185
### Non-Admin users
181186

182187
Non-admin users will require to be granted specific permissions to perform certain tasks and access resources.
@@ -540,7 +545,7 @@ do {
540545

541546
### Thread Safety
542547

543-
Every instance of the SDK `Client` is thread-safe. You **should** use the same instance throughout the entire lifecycle of your application. Each instance has its own Connection pool and Caching resources that are automatically released when the instance is garbage collected.
548+
Each instance of the SDK `Client` owns its own HTTP connection pool and cache. It is safe to reuse that instance on the same thread, but sharing it across multiple threads may leak some internal state. Follow the patterns in [Thread safety considerations](#thread-safety-considerations) to scope clients correctly. The underlying resources are released when the instance becomes eligible for garbage collection.
544549

545550
<a name="spring-support"></a>
546551
## Inject the Okta Java SDK in Spring

‎api/src/main/java/com/okta/sdk/client/MultiThreadingWarningUtil.java‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
/*
2+
* Copyright 2025-Present Okta, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.okta.sdk.client;
17+
18+
import org.slf4j.Logger;
19+
import org.slf4j.LoggerFactory;
20+
21+
import java.util.Set;
22+
import java.util.concurrent.ConcurrentHashMap;
23+
import java.util.concurrent.atomic.AtomicBoolean;
24+
import java.util.concurrent.atomic.AtomicInteger;
25+
26+
/**
27+
* Utility class to detect and warn about multi-threaded usage of the Okta SDK.
28+
*
29+
* <p>The Okta SDK stores per-thread pagination state keyed by thread ID. When multiple threads
30+
* use the same ApiClient instance, this can lead to:</p>
31+
* <ul>
32+
* <li>Unexpected pagination behavior (threads interfering with each other's pagination state)</li>
33+
* <li>Thread safety issues in thread pool environments</li>
34+
* <li>Difficult-to-debug race conditions</li>
35+
* </ul>
36+
*
37+
* <p>This utility tracks which threads access the SDK and emits warnings when multi-threaded
38+
* usage is detected. It is designed to have minimal performance impact and will only log
39+
* warnings, not prevent multi-threaded usage.</p>
40+
*
41+
*/
42+
public class MultiThreadingWarningUtil {
43+
44+
private static final Logger log = LoggerFactory.getLogger(MultiThreadingWarningUtil.class);
45+
46+
/**
47+
* Threshold for emitting warnings about the number of threads using the SDK.
48+
* If more than this many unique threads access the same ApiClient instance,
49+
* a warning will be logged.
50+
*/
51+
private static final int THREAD_COUNT_WARNING_THRESHOLD = 3;
52+
53+
/**
54+
* Maximum number of thread IDs to track before we stop tracking new ones.
55+
* This prevents unbounded memory growth in the tracking set itself.
56+
*/
57+
private static final int MAX_TRACKED_THREADS = 1000;
58+
59+
/**
60+
* Set of thread IDs that have accessed this ApiClient instance.
61+
* We use a ConcurrentHashMap as a thread-safe set.
62+
*/
63+
private final Set<Long> accessedThreadIds = ConcurrentHashMap.newKeySet();
64+
65+
/**
66+
* Counter for the number of unique threads that have accessed this ApiClient.
67+
*/
68+
private final AtomicInteger uniqueThreadCount = new AtomicInteger(0);
69+
70+
/**
71+
* Flag to track if we've already emitted the initial multi-threading warning.
72+
* We only want to warn once, not on every access.
73+
*/
74+
private final AtomicBoolean multiThreadWarningEmitted = new AtomicBoolean(false);
75+
76+
/**
77+
* Flag to track if we've already emitted the thread pool warning.
78+
*/
79+
private final AtomicBoolean threadPoolWarningEmitted = new AtomicBoolean(false);
80+
81+
/**
82+
* Flag to track if we've reached the max tracked threads limit.
83+
*/
84+
private final AtomicBoolean maxThreadsReachedWarningEmitted = new AtomicBoolean(false);
85+
86+
/**
87+
* Records that the current thread has accessed the SDK and checks if warnings should be emitted.
88+
*
89+
* <p>This method should be called at key points in the SDK lifecycle where thread state is
90+
* being used (e.g., when storing or retrieving pagination state).</p>
91+
*/
92+
public void recordThreadAccess() {
93+
long currentThreadId = Thread.currentThread().getId();
94+
String currentThreadName = Thread.currentThread().getName();
95+
96+
if (uniqueThreadCount.get() >= MAX_TRACKED_THREADS) {
97+
if (maxThreadsReachedWarningEmitted.compareAndSet(false, true)) {
98+
log.warn(
99+
"OKTA SDK WARNING: Maximum tracked threads ({}) exceeded. " +
100+
"Stopping thread tracking to prevent memory issues. " +
101+
"This indicates excessive thread churn. " +
102+
"The Okta SDK is not designed for high-concurrency, multi-threaded usage with a single ApiClient instance.",
103+
MAX_TRACKED_THREADS
104+
);
105+
}
106+
return;
107+
}
108+
109+
if (!accessedThreadIds.add(currentThreadId)) {
110+
return;
111+
}
112+
113+
int threadCount = uniqueThreadCount.getAndUpdate(count ->
114+
count >= MAX_TRACKED_THREADS ? count : count + 1
115+
);
116+
117+
if (threadCount >= MAX_TRACKED_THREADS) {
118+
accessedThreadIds.remove(currentThreadId);
119+
if (maxThreadsReachedWarningEmitted.compareAndSet(false, true)) {
120+
log.warn(
121+
"OKTA SDK WARNING: Maximum tracked threads ({}) exceeded. " +
122+
"Stopping thread tracking to prevent memory issues. " +
123+
"This indicates excessive thread churn. " +
124+
"The Okta SDK is not designed for high-concurrency, multi-threaded usage with a single ApiClient instance.",
125+
MAX_TRACKED_THREADS
126+
);
127+
}
128+
return;
129+
}
130+
131+
threadCount += 1;
132+
133+
log.debug("New thread detected accessing Okta SDK: {} (ID: {}). Total unique threads: {}",
134+
currentThreadName, currentThreadId, threadCount);
135+
136+
if (threadCount > THREAD_COUNT_WARNING_THRESHOLD) {
137+
if (multiThreadWarningEmitted.compareAndSet(false, true)) {
138+
emitMultiThreadingWarning(threadCount);
139+
}
140+
}
141+
142+
if (threadCount > 1 && isLikelyThreadPoolThread(currentThreadName)) {
143+
if (threadPoolWarningEmitted.compareAndSet(false, true)) {
144+
emitThreadPoolWarning();
145+
}
146+
}
147+
}
148+
149+
/**
150+
* Returns the number of unique threads that have accessed this ApiClient instance.
151+
*
152+
* @return the count of unique threads
153+
*/
154+
public int getUniqueThreadCount() {
155+
return uniqueThreadCount.get();
156+
}
157+
158+
/**
159+
* Checks if a thread name suggests it's from a thread pool.
160+
* Common patterns include names like "pool-1-thread-5", "http-nio-8080-exec-3",
161+
* "ForkJoinPool.commonPool-worker-1", etc.
162+
*
163+
* @param threadName the name of the thread
164+
* @return true if the thread name suggests it's from a thread pool
165+
*/
166+
private boolean isLikelyThreadPoolThread(String threadName) {
167+
if (threadName == null) {
168+
return false;
169+
}
170+
171+
String lowerName = threadName.toLowerCase();
172+
return lowerName.contains("pool") ||
173+
lowerName.contains("worker") ||
174+
lowerName.contains("executor") ||
175+
lowerName.contains("thread-") ||
176+
lowerName.contains("exec-");
177+
}
178+
179+
/**
180+
* Emits a warning about multi-threaded usage of the SDK.
181+
*/
182+
private void emitMultiThreadingWarning(int threadCount) {
183+
log.warn(
184+
"\n" +
185+
"================================================================================\n" +
186+
"OKTA SDK MULTI-THREADING WARNING\n" +
187+
"================================================================================\n" +
188+
"The Okta SDK has detected that {} unique threads are accessing the same\n" +
189+
"ApiClient instance. The SDK stores pagination metadata per thread and is\n" +
190+
"NOT designed for concurrent, multi-threaded usage patterns.\n" +
191+
"\n" +
192+
"POTENTIAL ISSUES:\n" +
193+
" - Pagination may behave unexpectedly when multiple threads make requests\n" +
194+
" - Thread state may interfere between concurrent operations\n" +
195+
" - In thread pool environments, per-thread state persists across requests\n" +
196+
"\n" +
197+
"RECOMMENDATIONS:\n" +
198+
" 1. Use a separate ApiClient instance per thread (thread-local pattern)\n" +
199+
" 2. Synchronize access to a shared ApiClient instance (not recommended for\n" +
200+
" high-concurrency scenarios due to performance impact)\n" +
201+
" 3. Avoid using the SDK's collection/pagination methods in multi-threaded\n" +
202+
" contexts where threads share an ApiClient instance\n" +
203+
"\n" +
204+
"For more information, see: https://github.com/okta/okta-sdk-java/issues/1637\n" +
205+
"================================================================================\n",
206+
threadCount
207+
);
208+
}
209+
210+
/**
211+
* Emits a warning specific to thread pool usage.
212+
*/
213+
private void emitThreadPoolWarning() {
214+
log.warn(
215+
"\n" +
216+
"================================================================================\n" +
217+
"OKTA SDK THREAD POOL WARNING\n" +
218+
"================================================================================\n" +
219+
"The Okta SDK has detected thread pool usage (e.g., in a web server or\n" +
220+
"async framework). This usage pattern has specific concerns:\n" +
221+
"\n" +
222+
"THREAD POOL ISSUE:\n" +
223+
" - Thread pool threads are reused and live for the lifetime of the application\n" +
224+
" - Per-thread state from one request can leak into subsequent requests\n" +
225+
" handled by the same thread\n" +
226+
" - Pagination state from Request A might unexpectedly affect Request B\n" +
227+
"\n" +
228+
"RECOMMENDED PATTERNS FOR WEB SERVERS:\n" +
229+
" 1. Create a new ApiClient per request (safest, but has overhead)\n" +
230+
" 2. Use a thread-local ApiClient pattern:\n" +
231+
" \n" +
232+
" private static final ThreadLocal<ApiClient> CLIENT_HOLDER =\n" +
233+
" ThreadLocal.withInitial(() -> createNewApiClient());\n" +
234+
" \n" +
235+
" 3. If using Spring, consider request-scoped beans\n" +
236+
" 4. Avoid storing pagination state across HTTP requests\n" +
237+
"\n" +
238+
"For more information, see: https://github.com/okta/okta-sdk-java/issues/1637\n" +
239+
"================================================================================\n"
240+
);
241+
}
242+
243+
/**
244+
* Resets all tracking state. This is primarily useful for testing.
245+
*
246+
* <p><strong>WARNING:</strong> This method should not be called in production code.
247+
* It exists solely for testing purposes.</p>
248+
*/
249+
public void reset() {
250+
accessedThreadIds.clear();
251+
uniqueThreadCount.set(0);
252+
multiThreadWarningEmitted.set(false);
253+
threadPoolWarningEmitted.set(false);
254+
maxThreadsReachedWarningEmitted.set(false);
255+
}
256+
}

0 commit comments

Comments
 (0)