From 18e8e22600be553b73465977705befb4d1e0342c Mon Sep 17 00:00:00 2001
From: Edward Oakes <ed.nmi.oakes@gmail.com>
Date: Wed, 10 Jan 2024 12:35:44 -0600
Subject: [PATCH] [serve] Remove `aiorwlock` dependency (#42159)

This was added long ago to avoid race conditions between reconfigure and user method calls. However, sometime since then the reader lock acquisition seems to have been accidentally removed. We have not seen any issues from this so my preference is to just altogether remove the complexity & dependency.

Users can implement their own coordination if this is a problem for them by adding concurrency control to their reconfigure method.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
---
 python/ray/serve/_private/replica.py | 36 +++++++++++++---------------
 python/setup.py                      |  1 -
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/python/ray/serve/_private/replica.py b/python/ray/serve/_private/replica.py
index b1e376a2a6a7..c969fa406d5d 100644
--- a/python/ray/serve/_private/replica.py
+++ b/python/ray/serve/_private/replica.py
@@ -9,7 +9,6 @@
 from importlib import import_module
 from typing import Any, AsyncGenerator, Callable, Dict, Optional, Tuple, Union
 
-import aiorwlock
 import starlette.responses
 from starlette.requests import Request
 from starlette.types import Message, Receive, Scope, Send
@@ -708,7 +707,6 @@ def __init__(
         self._init_kwargs = init_kwargs
         self._is_function = inspect.isfunction(deployment_def)
         self._deployment_id = deployment_id
-        self._rwlock = aiorwlock.RWLock()
         self._delete_lock = asyncio.Lock()
 
         # Will be populated in `initialize_callable`.
@@ -800,24 +798,24 @@ async def call_user_health_check(self):
         await self._user_health_check()
 
     async def call_reconfigure(self, user_config: Any):
-        async with self._rwlock.writer:
-            if user_config is not None:
-                if self._is_function:
-                    raise ValueError(
-                        "deployment_def must be a class to use user_config"
-                    )
-                elif not hasattr(self._callable, RECONFIGURE_METHOD):
-                    raise RayServeException(
-                        "user_config specified but deployment "
-                        + self._deployment_id
-                        + " missing "
-                        + RECONFIGURE_METHOD
-                        + " method"
-                    )
-                reconfigure_method = sync_to_async(
-                    getattr(self._callable, RECONFIGURE_METHOD)
+        # NOTE(edoakes): there is the possibility of a race condition in user code if
+        # they don't have any form of concurrency control between `reconfigure` and
+        # other methods. See https://github.com/ray-project/ray/pull/42159.
+        if user_config is not None:
+            if self._is_function:
+                raise ValueError("deployment_def must be a class to use user_config")
+            elif not hasattr(self._callable, RECONFIGURE_METHOD):
+                raise RayServeException(
+                    "user_config specified but deployment "
+                    + self._deployment_id
+                    + " missing "
+                    + RECONFIGURE_METHOD
+                    + " method"
                 )
-                await reconfigure_method(user_config)
+            reconfigure_method = sync_to_async(
+                getattr(self._callable, RECONFIGURE_METHOD)
+            )
+            await reconfigure_method(user_config)
 
     async def call_user_method(
         self,
diff --git a/python/setup.py b/python/setup.py
index 081cdd922964..b4a67058b872 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -267,7 +267,6 @@ def get_packages(self):
             "requests",
             "starlette",
             "fastapi",
-            "aiorwlock",
             "watchfiles",
         ],
         "tune": ["pandas", "tensorboardX>=1.9", "requests", pyarrow_dep, "fsspec"],