Skip to content

python: release GIL while calling terminate to avoid deadlock#1480

Closed
crockeo wants to merge 1 commit intomainfrom
ch/remove-python-deadlocks
Closed

python: release GIL while calling terminate to avoid deadlock#1480
crockeo wants to merge 1 commit intomainfrom
ch/remove-python-deadlocks

Conversation

@crockeo
Copy link
Contributor

@crockeo crockeo commented May 18, 2021

Description: Alternative to #1379 for getting rid of the deadlocks. The deadlocks are caused by:

  • Thread 1 calls Engine::terminate with GIL in hand, asks Thread 2, and then waits for it to die.
  • Thread 2 (envoy main thread) begins teardown, which causes EngineCallbacks::~EngineCallbacks to run, which attempts to grab the GIL.
  • Thread 1 is waiting for Thread 2 to die
  • Thread 2 is waiting for Thread 1 to give up the GIL

Risk Level: Low

Testing: ...a little convoluted. These deadlocks occur at process teardown, so I've been installing the envoy-requests wheel and making a pair of files:

# named test.py
import envoy_engine
import envoy_requests
from envoy_requests.common.engine import Engine

if __name__ == "__main__":
    Engine.log_level = envoy_engine.LogLevel.Debug
    envoy_requests.get("https://api.lyft.com/ping")

and

#!/usr/bin/env bash

while true; do
    python test.py
    sleep 0.5
done

and then running several of these until a deadlock occurs. You can also reproduce them by running several test jobs at the same time (--runs_per_test), but I found the above method to result in a deadlock sooner.

Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Cerek Hillen <chillen@lyft.com>
@crockeo
Copy link
Contributor Author

crockeo commented May 21, 2021

unnecessary with #1379 merged

@crockeo crockeo closed this May 21, 2021
@crockeo crockeo deleted the ch/remove-python-deadlocks branch May 27, 2021 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants