Consensys / teku

Open-source Ethereum consensus client written in Java
https://consensys.io/teku
Apache License 2.0
648 stars 263 forks source link

AsyncRunnerEventThread should complete futures off the event thread #4645

Open ajsutton opened 2 years ago

ajsutton commented 2 years ago

Description

AsyncRunnerEventThread provides an event thread where each "event" (or task) is executed on a single thread. It's vital that the single event thread is not blocked and we should limit what runs on it to just the actual events.

Currently though, when an event returns a value, it's done via a SafeFuture which is completed on the event thread. This means that any chained handlers (e.g via .thenApply) are also executed on the event thread even though they won't be designed to avoid blocking.

We should modify AsyncRunnerEventThread to ensure that any returned futures trigger handlers in a separate thread pool. This would likely be something similar to how AsyncEventDeliverer ensures EventChannel responses are delivered in a specific thread pool (via the AsyncRunner responseRunner it's passed).

rolfyone commented 1 year ago

Definitely still a valid ticket, will leave open.