FastChat icon indicating copy to clipboard operation
FastChat copied to clipboard

Change the controller heart beat thread to daemon

Open tumbarumba opened this issue 1 year ago • 0 comments

Why are these changes needed?

The controller creates a heart beat thread to remove stale workers on expiration. Currently, the thread is not flagged as daemon. In usual usage when stated by main, this is not a problem. However, if the controller is invoked by some other framework, the extra thread can stop a clean shutdown, as the Python interpreter will keep running while there are other non-daemon threads still alive.

An example of this is pytest. The following code will cause pytest to hang:

import sys
import fastchat.serve.controller as fastchat_controller
from fastapi.testclient import TestClient
from pytest import fixture
from starlette.status import HTTP_200_OK

@fixture
def test_client() -> TestClient:
    sys.argv = [sys.argv[0]]
    args, controller = fastchat_controller.create_controller()
    fastchat_controller.controller = controller
    return TestClient(fastchat_controller.app)

def test_get_models(test_client: TestClient) -> None:
    response = test_client.post("/list_models")
    assert response.status_code == HTTP_200_OK
    assert response.json() == {"models": []}

This change will allow the above code to run successfully.

Related issue number

Closes #3089

Checks

  • [x] I've run format.sh to lint the changes in this PR.
  • [x] I've included any doc changes needed.
  • [x] I've made sure the relevant tests are passing (if applicable).

tumbarumba avatar Mar 27 '24 23:03 tumbarumba