Skip to content

Commit 3ab123c

Browse files
stainless-botStainless Bot
authored andcommitted
feat(client): clean up resource leaks when the resource becomes phantom reachable
chore: unknown commit message
1 parent 685553f commit 3ab123c

File tree

6 files changed

+127
-11
lines changed

6 files changed

+127
-11
lines changed

openai-java-core/src/main/kotlin/com/openai/core/ClientOptions.kt

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper
66
import com.google.common.collect.ArrayListMultimap
77
import com.google.common.collect.ListMultimap
88
import com.openai.core.http.HttpClient
9+
import com.openai.core.http.PhantomReachableClosingHttpClient
910
import com.openai.core.http.RetryingHttpClient
1011
import java.time.Clock
1112

@@ -162,11 +163,13 @@ private constructor(
162163

163164
return ClientOptions(
164165
httpClient!!,
165-
RetryingHttpClient.builder()
166-
.httpClient(httpClient!!)
167-
.clock(clock)
168-
.maxRetries(maxRetries)
169-
.build(),
166+
PhantomReachableClosingHttpClient(
167+
RetryingHttpClient.builder()
168+
.httpClient(httpClient!!)
169+
.clock(clock)
170+
.maxRetries(maxRetries)
171+
.build()
172+
),
170173
jsonMapper ?: jsonMapper(),
171174
clock,
172175
baseUrl,
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
@file:JvmName("PhantomReachable")
2+
3+
package com.openai.core
4+
5+
import com.openai.errors.OpenAIException
6+
import java.lang.reflect.InvocationTargetException
7+
8+
/**
9+
* Closes [closeable] when [observed] becomes only phantom reachable.
10+
*
11+
* This is a wrapper around a Java 9+ [java.lang.ref.Cleaner], or a no-op in older Java versions.
12+
*/
13+
@JvmSynthetic
14+
internal fun closeWhenPhantomReachable(observed: Any, closeable: AutoCloseable) {
15+
check(observed !== closeable) {
16+
"`observed` cannot be the same object as `closeable` because it would never become phantom reachable"
17+
}
18+
closeWhenPhantomReachable?.let { it(observed, closeable::close) }
19+
}
20+
21+
private val closeWhenPhantomReachable: ((Any, AutoCloseable) -> Unit)? by lazy {
22+
try {
23+
val cleanerClass = Class.forName("java.lang.ref.Cleaner")
24+
val cleanerCreate = cleanerClass.getMethod("create")
25+
val cleanerRegister =
26+
cleanerClass.getMethod("register", Any::class.java, Runnable::class.java)
27+
val cleanerObject = cleanerCreate.invoke(null);
28+
29+
{ observed, closeable ->
30+
try {
31+
cleanerRegister.invoke(cleanerObject, observed, Runnable { closeable.close() })
32+
} catch (e: ReflectiveOperationException) {
33+
if (e is InvocationTargetException) {
34+
when (val cause = e.cause) {
35+
is RuntimeException,
36+
is Error -> throw cause
37+
}
38+
}
39+
throw OpenAIException("Unexpected reflective invocation failure", e)
40+
}
41+
}
42+
} catch (e: ReflectiveOperationException) {
43+
// We're running Java 8, which has no Cleaner.
44+
null
45+
}
46+
}

openai-java-core/src/main/kotlin/com/openai/core/handlers/SseHandler.kt

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper
66
import com.openai.core.JsonValue
77
import com.openai.core.http.HttpResponse
88
import com.openai.core.http.HttpResponse.Handler
9+
import com.openai.core.http.PhantomReachableClosingStreamResponse
910
import com.openai.core.http.SseMessage
1011
import com.openai.core.http.StreamResponse
1112
import com.openai.errors.OpenAIException
@@ -58,14 +59,16 @@ internal fun sseHandler(jsonMapper: JsonMapper): Handler<StreamResponse<SseMessa
5859
}
5960
}
6061

61-
return object : StreamResponse<SseMessage> {
62-
override fun stream(): Stream<SseMessage> = sequence.asStream()
62+
return PhantomReachableClosingStreamResponse(
63+
object : StreamResponse<SseMessage> {
64+
override fun stream(): Stream<SseMessage> = sequence.asStream()
6365

64-
override fun close() {
65-
reader.close()
66-
response.close()
66+
override fun close() {
67+
reader.close()
68+
response.close()
69+
}
6770
}
68-
}
71+
)
6972
}
7073
}
7174

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package com.openai.core.http
2+
3+
import com.openai.core.RequestOptions
4+
import com.openai.core.closeWhenPhantomReachable
5+
import java.util.concurrent.CompletableFuture
6+
7+
internal class PhantomReachableClosingHttpClient(private val httpClient: HttpClient) : HttpClient {
8+
init {
9+
closeWhenPhantomReachable(this, httpClient)
10+
}
11+
12+
override fun execute(request: HttpRequest, requestOptions: RequestOptions): HttpResponse =
13+
httpClient.execute(request, requestOptions)
14+
15+
override fun executeAsync(
16+
request: HttpRequest,
17+
requestOptions: RequestOptions
18+
): CompletableFuture<HttpResponse> = httpClient.executeAsync(request, requestOptions)
19+
20+
override fun close() = httpClient.close()
21+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.openai.core.http
2+
3+
import com.openai.core.closeWhenPhantomReachable
4+
import java.util.stream.Stream
5+
6+
internal class PhantomReachableClosingStreamResponse<T>(
7+
private val streamResponse: StreamResponse<T>
8+
) : StreamResponse<T> {
9+
init {
10+
closeWhenPhantomReachable(this, streamResponse)
11+
}
12+
13+
override fun stream(): Stream<T> = streamResponse.stream()
14+
15+
override fun close() = streamResponse.close()
16+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.openai.core
2+
3+
import org.assertj.core.api.Assertions.assertThat
4+
import org.junit.jupiter.api.Test
5+
6+
internal class PhantomReachableTest {
7+
8+
@Test
9+
fun closeWhenPhantomReachable_whenObservedIsGarbageCollected_closesCloseable() {
10+
var closed = false
11+
val closeable = AutoCloseable { closed = true }
12+
13+
closeWhenPhantomReachable(
14+
// Pass an inline object for the object to observe so that it becomes immediately
15+
// unreachable.
16+
Any(),
17+
closeable
18+
)
19+
20+
assertThat(closed).isFalse()
21+
22+
System.gc()
23+
Thread.sleep(3000)
24+
25+
assertThat(closed).isTrue()
26+
}
27+
}

0 commit comments

Comments
 (0)