lru.LRU is not thread-safe; session cache maintenance (web3._utils.request) can crash the interpreter

[jamadden]

• Version: 5.15.0, but current master too.
• Python: Any
• OS: Any

What was wrong?

In #876, code was added to close sessions as they are evicted from the session cache. But in the presence of multiple threads this can lead to interpreter crashes, because lru.LRU is not thread safe. If, while running the callback function a thread switch happens, and that other thread also attempts to add a new session or otherwise manipulate the session cache, the interpreter can crash. (When we re-enter the lru.LRU object, the internal state of the object is inconsistent while the callback is running. Because the callback is arbitrary Python code, it can easily drop the GIL.)

This was originally reported in rotki/rotki#2132. There, they're using greenlets, but its easy to demonstrate the crash also using threads (a sample script is in that issue).

How can it be fixed?

Adding a lock around uses of _session_cache ought to fix the problem.

[kclowes]

Wow, thanks for the report @jamadden! Will put this near the top of our todo list.

[Eenae]

The issue occurred in our setup (> 8 threads, various endpoints) quite stably. Synchronizing the cache access indeed solved the problem.

[kclowes]

@Eenae when you say:

Synchronizing the cache access indeed solved the problem.

Do you mean PR #2407 fixed your issue?

[Eenae]

@kclowes no, it seems #2407 is not a PR.
Did it via a monkey-patch:

from typing import Callable
import threading
import web3._utils.request

lock = threading.Lock()

def locked(fn: Callable) -> Callable:
    def inner(*args, **kwargs):
        with lock:
            return fn(*args, **kwargs)
    return inner

web3._utils.request.cache_session = locked(web3._utils.request.cache_session)
web3._utils.request._get_session = locked(web3._utils.request._get_session)

[kclowes]

Oops, I meant #2409. I think that's about equivalent to what you have. Thanks for the example!

[fselmo]

Resolved in #2409 for web3.py v6. Needs a back-port to v5 PR.

[LefterisJP]

Hey @fselmo sorry for the ping but has this been backported to v5? And if yet in which release?

[fselmo]

Hey @LefterisJP, this was added to v5 in #2691 but there is not yet a v5 version out with these changes in, it only exists on the master branch as of now. I'll see if we can change that asap. Thanks for the ping!

edit: Just released in web3.py v5.31.2

[LefterisJP]

Thank you so much @fselmo. That was a really fast response and action. Appreciate it!