From c4b85602477dfd7c6e14dc0236b63c2653baeaab Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 27 Dec 2024 12:55:32 +0530 Subject: [PATCH] fix: Ensure only one Redis connection is created (#28930) There is no gaurantee that setup_cache is only called once. This PR adds a mutex lock to ensure only one thread gets to create the connection. If both arrive at same time then one of them will be blocked until connection is setup. So far this hasn't been an issue because the "orphan" connection would just get garbage collected but if you setup any kind of listener on it or refer to it then it will keep running forever hurting performance. This just has small performance impact on first request that sets up the connection, in absence of contention the lock should have almost no overhead. I make up for it by eliminating one function call :pinch: --- frappe/__init__.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 53519b4b83..8ccf5b184c 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -18,6 +18,7 @@ import inspect import json import os import sys +import threading import traceback import warnings from collections import defaultdict @@ -272,7 +273,8 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force: bool = local.dev_server = _dev_server local.qb = get_query_builder(local.conf.db_type) local.qb.get_query = get_query - setup_redis_cache_connection() + if not cache: + setup_redis_cache_connection() if not _one_time_setup.get(local.conf.db_type): patch_query_execute() @@ -490,14 +492,19 @@ def destroy(): release_local(local) +_redis_init_lock = threading.Lock() + + def setup_redis_cache_connection(): """Defines `frappe.cache` as `RedisWrapper` instance""" + from frappe.utils.redis_wrapper import setup_cache + global cache - if not cache: - from frappe.utils.redis_wrapper import setup_cache - - cache = setup_cache() + with _redis_init_lock: + # We need to check again since someone else might have setup connection before us. + if not cache: + cache = setup_cache() def get_traceback(with_context: bool = False) -> str: