Skip to content
This repository was archived by the owner on Aug 23, 2022. It is now read-only.

Conversation

@k0nserv
Copy link
Member

@k0nserv k0nserv commented Jun 17, 2022

All the timegen code was async because it used an async
lock(tokio::sync::Mutex). However, there's no need to use such a lock
and doing so makes it more complex to achieve cancel safety elsewhere.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #5 (6e1af04) into main (07222b3) will decrease coverage by 28.99%.
The diff coverage is 96.49%.

@@             Coverage Diff             @@
##             main       #5       +/-   ##
===========================================
- Coverage   59.81%   30.81%   -29.00%     
===========================================
  Files          32       32               
  Lines        2013     4524     +2511     
  Branches      575     1180      +605     
===========================================
+ Hits         1204     1394      +190     
- Misses        250     2571     +2321     
  Partials      559      559               
Impacted Files Coverage Δ
src/nack/generator/mod.rs 0.00% <ø> (ø)
src/report/mod.rs 90.47% <ø> (ø)
src/report/receiver/mod.rs 100.00% <ø> (ø)
src/report/sender/mod.rs 0.00% <ø> (ø)
src/mock/mock_time.rs 75.00% <66.66%> (+17.85%) ⬆️
src/nack/generator/generator_stream.rs 89.13% <100.00%> (+5.43%) ⬆️
src/report/receiver/receiver_stream.rs 84.61% <100.00%> (+6.23%) ⬆️
src/report/receiver/receiver_test.rs 67.08% <100.00%> (+0.26%) ⬆️
src/report/sender/sender_stream.rs 76.19% <100.00%> (+2.38%) ⬆️
src/report/sender/sender_test.rs 67.74% <100.00%> (+3.77%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07222b3...6e1af04. Read the comment docs.

This makes the following methods cancel safe:

* `report::receiver::receiver_stream::Receiverstream::read`
* `nack::generator::generator_stream::GeneratorStream::read`

This was achieved mostly by making locks(`tokio::Mutex`) that were async
not be. Since none of these locks are held across `.await` points
there's no need for them to be async locks.
@k0nserv k0nserv force-pushed the fix/de-async-time-gen branch from 7c37dc9 to 6e1af04 Compare June 17, 2022 13:52
@k0nserv k0nserv changed the title Remove async from time gen Improve cancel saftety of RTP interceptors Jun 17, 2022
@k0nserv k0nserv merged commit 7421233 into main Jun 17, 2022
@k0nserv k0nserv deleted the fix/de-async-time-gen branch June 17, 2022 14:40
@k0nserv k0nserv mentioned this pull request Jun 20, 2022
14 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants