Skip to content

Commit 14dda84

Browse files
committed
fix(media): don't hang flush() on media logged after shutdown()
shutdown() stops the media upload consumers, but a client used afterwards (the same client, Langfuse(...) with the same key, or get_client()) still queues media found in span input, output and metadata. No consumer takes it, so flush() and every later shutdown() block forever in Queue.join(). Media logged while shutdown() stops the consumers can be stranded the same way. shutdown() now stops accepting media right after its own flush(), uploads what was queued since, then stops the consumers. The flag check and put() share a lock, so nothing can be queued after the consumers stop. Later media is skipped with a warning, like scores created after shutdown().
1 parent 1eb1be3 commit 14dda84

3 files changed

Lines changed: 118 additions & 5 deletions

File tree

‎langfuse/_client/resource_manager.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,8 @@ def _at_fork_reinit(self) -> None:
410410
# the lock is class-level state needed by the child (e.g. to create a new client)
411411
# even if this particular instance was already shut down.
412412
LangfuseResourceManager._lock = threading.RLock()
413+
# Media logged after shutdown() still takes this lock.
414+
self._media_manager._enqueue_lock = threading.RLock()
413415

414416
if self._shutdown:
415417
return
@@ -617,6 +619,11 @@ def shutdown(self) -> None:
617619
atexit.unregister(self.shutdown)
618620

619621
self.flush()
622+
# No consumer would take media queued after the consumers stop, and
623+
# flush() would then wait for it forever. Upload what was queued since
624+
# the flush() above before stopping them.
625+
self._media_manager.stop_accepting_uploads()
626+
self._media_upload_queue.join()
620627
self._stop_and_join_consumer_threads()
621628

622629

‎langfuse/_task_manager/media_manager.py‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import os
2+
import threading
23
import time
34
from queue import Empty, Full, Queue
45
from typing import Any, Callable, Optional, TypeVar, cast
@@ -45,6 +46,10 @@ def __init__(
4546
self._enabled = os.environ.get(
4647
LANGFUSE_MEDIA_UPLOAD_ENABLED, "True"
4748
).lower() not in ("false", "0")
49+
# Reentrant, so shutdown() from a signal handler does not deadlock when
50+
# the interrupted thread holds it.
51+
self._enqueue_lock = threading.RLock()
52+
self._accepting_uploads = True
4853

4954
def reinitialize(
5055
self,
@@ -57,6 +62,11 @@ def reinitialize(
5762
self._httpx_client = httpx_client
5863
self._queue = media_upload_queue
5964

65+
def stop_accepting_uploads(self) -> None:
66+
"""Skip media found from now on, as the consumers are about to stop."""
67+
with self._enqueue_lock:
68+
self._accepting_uploads = False
69+
6070
def process_next_media_upload(self) -> None:
6171
try:
6272
upload_job = self._queue.get(block=True, timeout=1)
@@ -343,10 +353,23 @@ def _process_media(
343353
field=field,
344354
)
345355

346-
self._queue.put(
347-
item=upload_media_job,
348-
block=False,
349-
)
356+
with self._enqueue_lock:
357+
accepting_uploads = self._accepting_uploads
358+
if accepting_uploads:
359+
self._queue.put(
360+
item=upload_media_job,
361+
block=False,
362+
)
363+
364+
if not accepting_uploads:
365+
logger.warning(
366+
"Media upload skipped: Langfuse client has been shut down. "
367+
"media_id=%s trace_id=%s",
368+
media._media_id,
369+
trace_id,
370+
)
371+
return
372+
350373
logger.debug(
351374
"Queue: Enqueued media ID %s for upload processing | trace_id=%s | "
352375
"field=%s",

‎tests/unit/test_resource_manager.py‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22

33
import logging
44
import threading
5+
import time
56
from queue import Queue
67
from types import SimpleNamespace
78
from typing import Sequence
89
from unittest.mock import Mock
910

11+
import httpx
1012
import pytest
11-
from opentelemetry.sdk.trace import ReadableSpan
13+
from opentelemetry.sdk.trace import ReadableSpan, TracerProvider
1214
from opentelemetry.sdk.trace.export import SpanExporter, SpanExportResult
1315

1416
from langfuse import Langfuse
@@ -17,6 +19,7 @@
1719
from langfuse._task_manager.media_manager import MediaManager
1820
from langfuse._task_manager.media_upload_consumer import MediaUploadConsumer
1921
from langfuse._task_manager.score_consumer import ScoreConsumer
22+
from langfuse.media import LangfuseMedia
2023
from langfuse.types import MaskOtelSpansResult
2124

2225

@@ -226,6 +229,7 @@ def test_at_fork_reinit_skips_when_shutdown(monkeypatch):
226229

227230
old_score_queue = rm._score_queue
228231
old_lock = LangfuseResourceManager._lock
232+
old_media_lock = rm._media_manager._enqueue_lock
229233

230234
rm._shutdown = True
231235
rm._at_fork_reinit()
@@ -234,6 +238,7 @@ def test_at_fork_reinit_skips_when_shutdown(monkeypatch):
234238
# The class-level lock is shared state the child still needs (e.g. to create
235239
# a new client), so it must be replaced even when this instance is shut down.
236240
assert LangfuseResourceManager._lock is not old_lock
241+
assert rm._media_manager._enqueue_lock is not old_media_lock
237242

238243
client.shutdown()
239244

@@ -506,3 +511,81 @@ def test_flush_returns_after_repeated_shutdown(concurrent):
506511
media_queue.task_done()
507512

508513
assert not hung
514+
515+
516+
def _log_media(client: Langfuse, content: bytes) -> None:
517+
with client.start_as_current_observation(
518+
name="media",
519+
input=LangfuseMedia(content_bytes=content, content_type="image/png"),
520+
):
521+
pass
522+
523+
524+
@pytest.mark.filterwarnings("error::pytest.PytestUnhandledThreadExceptionWarning")
525+
def test_flush_returns_when_media_is_logged_after_shutdown(caplog):
526+
client = Langfuse(
527+
public_key="pk-media-after-shutdown",
528+
secret_key="sk-media-after-shutdown",
529+
span_exporter=NoOpSpanExporter(),
530+
tracer_provider=TracerProvider(),
531+
)
532+
media_queue = client._resources._media_upload_queue
533+
client.shutdown()
534+
with caplog.at_level(logging.WARNING, logger="langfuse"):
535+
_log_media(client, b"after-shutdown")
536+
537+
flush = threading.Thread(target=client.flush, daemon=True)
538+
flush.start()
539+
flush.join(timeout=2)
540+
hung = flush.is_alive()
541+
# Acknowledge media that no consumer took, so a hung flush() and the
542+
# reset() in the autouse fixture can finish.
543+
while media_queue.unfinished_tasks:
544+
media_queue.get_nowait()
545+
media_queue.task_done()
546+
547+
assert not hung
548+
assert any("shut down" in r.getMessage() for r in caplog.records)
549+
550+
551+
@pytest.mark.filterwarnings("error::pytest.PytestUnhandledThreadExceptionWarning")
552+
def test_shutdown_uploads_media_logged_after_its_flush(monkeypatch):
553+
media_requests = []
554+
555+
def handler(request: httpx.Request) -> httpx.Response:
556+
media_requests.append(request)
557+
# Keeps the consumer busy while shutdown() stops it.
558+
time.sleep(0.05)
559+
# No uploadUrl: the server already has the media, so the job ends here.
560+
return httpx.Response(200, json={"mediaId": "media-id", "uploadUrl": None})
561+
562+
client = Langfuse(
563+
public_key="pk-media-during-shutdown",
564+
secret_key="sk-media-during-shutdown",
565+
span_exporter=NoOpSpanExporter(),
566+
tracer_provider=TracerProvider(),
567+
httpx_client=httpx.Client(transport=httpx.MockTransport(handler)),
568+
)
569+
resources = client._resources
570+
media_queue = resources._media_upload_queue
571+
flush = resources.flush
572+
573+
def flush_then_log_media() -> None:
574+
flush()
575+
# Stands in for other threads logging media while shutdown() runs.
576+
_log_media(client, b"first")
577+
_log_media(client, b"second")
578+
579+
monkeypatch.setattr(resources, "flush", flush_then_log_media)
580+
shutdown = threading.Thread(target=client.shutdown, daemon=True)
581+
shutdown.start()
582+
shutdown.join(timeout=5)
583+
hung = shutdown.is_alive()
584+
# Acknowledge media that no consumer took, so a hung shutdown() and the
585+
# reset() in the autouse fixture can finish.
586+
while media_queue.unfinished_tasks:
587+
media_queue.get_nowait()
588+
media_queue.task_done()
589+
590+
assert not hung
591+
assert len(media_requests) == 2

0 commit comments

Comments
 (0)