Skip to content

Commit 550abb6

Browse files
walcz-delocalai-org-maint-bot
authored andcommitted
fix(backend/python): don't await sync servicer behaviors in AsyncModelIdentityInterceptor
The model-identity interceptor (added for #10952) is installed on every Python backend's gRPC server. Its grpc.aio variant invokes the wrapped servicer behavior itself and awaits the result unconditionally: result = await original(request, context) # LoadModel return await original_unary(request, context) # guarded RPCs async for response in original_stream(request, context): # streaming But a backend's servicer methods may be plain sync functions. The transformers backend, for one, defines `def LoadModel` and `def Embedding` (not `async def`). grpc.aio's own dispatch adapts both shapes, but this interceptor calls the behavior directly and bypasses that. For a sync method `original(...)` returns a message object, not a coroutine, so the `await` raises: TypeError: object Result can't be used in 'await' expression The model loads, then the LoadModel RPC dies on return; the guarded sync Embedding fails the same way. It happens on every platform, not just one backend build. CI never caught it because AsyncModelIdentityInterceptor had no behavioral test -- only an "is it installed" assertion. Fix: await only when the behavior actually returned an awaitable (inspect.isawaitable), mirroring grpc.aio's own sync/async adaptation. The streaming guard iterates a sync generator with `for` and an async one with `async for`. Adds async-path coverage to model_identity_test.py exercising both sync and async LoadModel / guarded-unary / streaming behaviors. The sync cases fail on the current code with the TypeError above and pass with this fix. Signed-off-by: stefanwalcz <stefan.walcz@walcz.de>
1 parent d0401f9 commit 550abb6

2 files changed

Lines changed: 126 additions & 4 deletions

File tree

backend/python/common/model_identity.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
the model itself.
2121
"""
2222

23+
import inspect
2324
import threading
2425

2526
import grpc
@@ -200,7 +201,14 @@ async def intercept_service(self, continuation, handler_call_details):
200201
original = handler.unary_unary
201202

202203
async def record(request, context):
203-
result = await original(request, context)
204+
# A backend's LoadModel may be a plain sync method (many define
205+
# `def LoadModel`, not `async def`). grpc.aio's own dispatch
206+
# adapts both, but this interceptor calls the behavior directly,
207+
# so it must not await a non-awaitable return -- otherwise a sync
208+
# backend fails with "object <T> can't be used in 'await'".
209+
result = original(request, context)
210+
if inspect.isawaitable(result):
211+
result = await result
204212
if getattr(result, "success", True):
205213
self.state.record(getattr(request, "Model", ""))
206214
return result
@@ -214,8 +222,15 @@ async def guard_stream(request, context):
214222
message = self.state.mismatch(getattr(request, "ModelIdentity", ""))
215223
if message is not None:
216224
await context.abort(grpc.StatusCode.NOT_FOUND, message)
217-
async for response in original_stream(request, context):
218-
yield response
225+
# A sync backend yields a plain generator, an async one an async
226+
# generator; iterate whichever this is.
227+
stream = original_stream(request, context)
228+
if hasattr(stream, "__aiter__"):
229+
async for response in stream:
230+
yield response
231+
else:
232+
for response in stream:
233+
yield response
219234

220235
return _rebuild(handler, guard_stream)
221236

@@ -225,6 +240,9 @@ async def guard(request, context):
225240
message = self.state.mismatch(getattr(request, "ModelIdentity", ""))
226241
if message is not None:
227242
await context.abort(grpc.StatusCode.NOT_FOUND, message)
228-
return await original_unary(request, context)
243+
result = original_unary(request, context)
244+
if inspect.isawaitable(result):
245+
result = await result
246+
return result
229247

230248
return _rebuild(handler, guard)

backend/python/common/model_identity_test.py

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
enforcement that rejects requests it should serve.
1010
"""
1111

12+
import asyncio
1213
import os
1314
import unittest
1415

@@ -302,6 +303,109 @@ def test_every_modality_rpc_serves_without_an_identity(self):
302303
def test_codec_rpcs_stay_unguarded(self):
303304
for method in ("/backend.Backend/AudioEncode", "/backend.Backend/AudioDecode"):
304305
self.assertNotIn(method, model_identity._GUARDED_METHODS)
306+
class TestAsyncInterceptorBehavior(unittest.TestCase):
307+
"""The grpc.aio counterpart, which had no behavioral coverage.
308+
309+
AsyncModelIdentityInterceptor wraps a backend's own servicer behavior. That
310+
behavior may be sync or async: several backends define `def LoadModel` and
311+
`def Embedding` (not `async def`), and grpc.aio's dispatch adapts both. The
312+
interceptor invokes the behavior itself, so if it awaits unconditionally it
313+
breaks every sync method it guards with "object <T> can't be used in
314+
'await'". These tests exercise both shapes; the sync ones are the regression.
315+
"""
316+
317+
def setUp(self):
318+
self.interceptor = model_identity.AsyncModelIdentityInterceptor()
319+
320+
def _wrap(self, method, handler):
321+
async def continuation(_):
322+
return handler
323+
324+
return asyncio.run(
325+
self.interceptor.intercept_service(continuation, _FakeCallDetails(method))
326+
)
327+
328+
def _load(self, behavior, model):
329+
wrapped = self._wrap("/backend.Backend/LoadModel", _handler(behavior))
330+
return asyncio.run(wrapped.unary_unary(_Request(Model=model), _FakeContext()))
331+
332+
def _call_unary(self, behavior, identity):
333+
wrapped = self._wrap("/backend.Backend/Predict", _handler(behavior))
334+
return asyncio.run(
335+
wrapped.unary_unary(_Request(ModelIdentity=identity), _FakeContext())
336+
)
337+
338+
def _drain_stream(self, behavior, identity):
339+
wrapped = self._wrap(
340+
"/backend.Backend/PredictStream", _handler(behavior, response_streaming=True)
341+
)
342+
343+
async def drain():
344+
out = []
345+
async for item in wrapped.unary_stream(
346+
_Request(ModelIdentity=identity), _FakeContext()
347+
):
348+
out.append(item)
349+
return out
350+
351+
return asyncio.run(drain())
352+
353+
# --- LoadModel: sync behavior is the regression, async must still work ---
354+
355+
def test_load_records_with_sync_behavior(self):
356+
self._load(lambda request, context: _Result(), "a.gguf")
357+
self.assertEqual(self.interceptor.state.loaded, "a.gguf")
358+
359+
def test_load_records_with_async_behavior(self):
360+
async def behavior(request, context):
361+
return _Result()
362+
363+
self._load(behavior, "a.gguf")
364+
self.assertEqual(self.interceptor.state.loaded, "a.gguf")
365+
366+
def test_failed_sync_load_records_nothing(self):
367+
self._load(lambda request, context: _Result(success=False), "a.gguf")
368+
self.assertEqual(self.interceptor.state.loaded, "")
369+
370+
# --- guarded unary: sync and async behaviors both served / rejected ---
371+
372+
def test_guard_serves_sync_behavior(self):
373+
self._load(lambda request, context: _Result(), "a.gguf")
374+
result = self._call_unary(lambda request, context: "served", "a.gguf")
375+
self.assertEqual(result, "served")
376+
377+
def test_guard_serves_async_behavior(self):
378+
self._load(lambda request, context: _Result(), "a.gguf")
379+
380+
async def behavior(request, context):
381+
return "served"
382+
383+
self.assertEqual(self._call_unary(behavior, "a.gguf"), "served")
384+
385+
def test_guard_rejects_mismatch(self):
386+
self._load(lambda request, context: _Result(), "a.gguf")
387+
with self.assertRaises(_Aborted):
388+
self._call_unary(lambda request, context: "served", "b.gguf")
389+
390+
# --- guarded stream: sync generator and async generator both work ---
391+
392+
def test_guard_stream_serves_sync_generator(self):
393+
self._load(lambda request, context: _Result(), "a.gguf")
394+
395+
def behavior(request, context):
396+
yield "a"
397+
yield "b"
398+
399+
self.assertEqual(self._drain_stream(behavior, "a.gguf"), ["a", "b"])
400+
401+
def test_guard_stream_serves_async_generator(self):
402+
self._load(lambda request, context: _Result(), "a.gguf")
403+
404+
async def behavior(request, context):
405+
yield "a"
406+
yield "b"
407+
408+
self.assertEqual(self._drain_stream(behavior, "a.gguf"), ["a", "b"])
305409

306410

307411
if __name__ == "__main__":

0 commit comments

Comments
 (0)