Threadshare: various fixes
This MR is a set of fixes further to discussions in !228 (closed).
block_on
futures::executor::block_on
executes a Future
on current thread. As the name suggests, this function blocks the thread, meaning that no other tasks on the same executor
can make progress. Not only does this defeats the purpose of the threadshare model, but it can also lead to deadlocks.
The first commit introduces a runtime::executor::block_on
function which allows blocking on a Future
provided it is NOT called under a Context
.
Edit: this modification is being discussed in: !240 (comment 378454)
Mixed pipelines
There was a limitation with some kinds of pipelines, like:
| ts-src |-->| queue |-->| ts-sink |
The ts-sink
received the PadContext
sticky event and added its pending tasks to the task queue. But ts-src
PadSrc::push*
functions returned before ts-sink
add a chance to do this. PadSink
now checks if the sink_chain*
& sink_event*
functions are executed under a Context
thread. If it's not the case, the returned Future
is executed immediately whether a PadContext
sticky event was received or not.
delay_for / interval
The Future
s returned by delay_for
and interval
must be instantiated under the executor in which they will be used. Since the Future
might not be resolved under a Context
eventually, these functions are now declared under runtime::time
. They no longer accept a closure returning a Future
as an argument and should be used like their counterparts in tokio. Note however that interval
returns a plain Stream
, so .next()
should be used in place of .tick()
. The delay_for
and interval
in runtime::time
are actually wrappers around the tokio equivalent but should be preferred over the tokio versions in order to reduce breakage if we decided to change the underlying runtime environment in the future.
JoinHandle
[Pad]Context::spawn
function now return a crate-defined JoinHandle
, which is also a wrapper around the underlying tokio JoinHandle
.
FlushStart & FlushStop
There were multiple problems with the implementations for FlushStart
& FlushStop
events handlers. FlushStop
was incorrectly handled as a serialized event and the implementations needed to rely on block_on
s. The solution consists in spawning a Future
to handle FlushStart
asynchronously. FlushStop
is now correctly handled with the buffer flow and await
s for FlushStart
to complete before executing.
Most block_on
calls where removed from the event handlers. These are the exceptions in current code base:
-
ProxySink
can't forward the non-serialized events to the pad of theProxySrc
directly and uses the shared queue, which currently uses an asyncMutex
. Fixing the pad problem should be enough to solve the problem. Edit: this is fixed in this MR. -
JitterBuffer
shares many data from the state & the settings which are used in thePad{Src,Sink}Handler
in non-async functions. I think a more in depth analysis of the data separation and selection of sync/asyncMutex
es orRwLock
s should be engaged. I think this exceeds the scope of this MR.
Edit: see limitations discussed in: !240 (comment 378454)
check_reconfigure
While porting the threadshare package to tokio-0.2.x
and introducing the Pad{Src,Sink}
model, I overlooked the internally-linked pad reconfiguration & PadContext
propagation in Queue
& Proxy
. This should be fixed now.
prepare_socket / complete_preparation
tokio's UdpSocket
& TcpStream
must be instantiated under their target executor. Due to throttling of the executor, if we Context::spawn
ed the instantiation and then await
ed in the element's prepare
function, we ended up waiting max_throttling_duration
for each ts-{UdpSrc,TcpClientSrc}-element in the pipeline. For the use cases targeted by the threadshare model, this lead to a looong NullToReady
transition. So we decided to use an approach with an async
prepare_socket
function that was spawned on the target Context
from the prepare
function. In a subsequent state transition, we executed complete_preparation
which purpose was to await
for prepare_socket
to complete.
A Context::enter
function was introduced which is similar to tokio::runtime::enter
. This function allows executing a function under the target executor. For UdpSocket
, this is all we need to revert back to instantiating under the prepare
function without noticeable delays for the NullToReady
transition. However, TcpStream::connect
is an async function. Since prepare
is already executing under a runtime, we can't block_on
TcpStream::connect
in Context::enter
, so I decided to keep the prepare_soket
/ complete_preparation
approach for ts-tcpclientsrc.