Deprecate BlockingTrioPortal in favor of from_thread.run and from_thread.run_sync

[epellis]

This addressses issue #810 by implementing trio.from_thread.run and trio.from_thread.run_sync and deprecating BlockingTrioPortal. There is probably some more stuff we need to add before this PR is fully complete, like integrating #1098 and the run_sync_soon API, but this is a good start.

Support re-entering the same Trio.run() loop by stashing the current
Trio Token in Thread Local Storage right before run_sync_in_thread
spawns a worker thread. When trio.from_thread.run_sync or
trio.from_thread.run are called in this thread, they can access the
token and use it to re-enter the Trio.run() loop.

This commit deprecates BlockingTrioPortal. For the majority of
use cases, using the thread local Trio Token should be sufficient. If
for any reason, another Trio Token needs to be used, it can be
passed as a kwarg to from_thread.run and from_thread.run_sync.

Here is a snippet from how the new API works:

import trio

def thread_fn():
    start = trio.from_thread.run_sync(trio.current_time)
    print("In Trio-land, the time is now:", start)
    trio.from_thread.run(trio.sleep, 1)
    end = trio.from_thread.run_sync(trio.current_time)
    print("And now it's:", end)

async def main():
    await trio.run_sync_in_thread(thread_fn)

trio.run(main)

Here is how the same code works in "old" Trio:

import trio

def thread_fn(token):
    portal = trio.BlockingTrioPortal(token)
    start = portal.run_sync(trio.current_time)
    print("In Trio-land, the time is now:", start)
    portal.run(trio.sleep, 1)
    end = portal.run_sync(trio.current_time)
    print("And now it's:", end)

async def main():
    token = trio.hazmat.current_trio_token()
    await trio.run_sync_in_thread(thread_fn, token)

trio.run(main)

[njsmith]

This is a great start! Making the trio_token an optional argument is probably better than the complicated thing that I was suggesting.

Some boring comments:

• We need to update the docs too. Generally when we deprecate something, we remove it from the docs immediately. (Anyone still using the old deprecated thing can look at the old docs if they really need them, and new users don't need to know about the old deprecated stuff.) So basically, you'll want to search for BlockingTrioPortal, and everywhere it shows up convert the docs to talk about trio.from_thread instead.

• We'll need a newsfragment to tell people about the change. In this case, we already have a 810.removal.rst, so I guess we'll want to update that... we can figure out the details here at the end once the rest is sorted out, but I wanted to let you know it was coming :-).

Regarding the code itself:

• We have a bunch of tests that use BlockingTrioPortal incidentally, or that are testing edge cases of BlockingTrioPortal. We should switch these to test trio.from_thread instead! Basically just a search-and-replace job I guess.

• Once we've done that, codecov will complain that we've got lots of untested code in BlockingTrioPortal. To keep duplication down, we might as well delete all that code and switch it to using trio.from_thread instead... like, BlockingTrioPortal.__init__ will still want to capture a trio token, and then BlockingTrioPortal.run will just be a call to from_thread.run(..., trio_token=self._token), and ditto for run_sync.

Let us know if you have any questions!

[njsmith]

ReStrustructured Text format is a little weird: double-backticks, like ``trio.from_thread.run``, mean raw literal code – it's the equivalent of markdown's single-backticks. If you want to use :func: to link to a named function, then you have to use single-backticks: :func:`trio.from_thread.run`.

But, since #1091 was merged recently, we don't even need to write :func: most of the time – if you just plain single-backticks, like `trio.from_thread.run`, then sphinx will now automatically find the object and link to it – you don't have to use :func: to specify the type.

[epellis]

Thanks for the hint! Yeah, I have never used RST, only Markdown, and also never Sphinx either so the docs are a bit of a learning curve for me.

[njsmith]

It looks like these double backticks still need to be replaced with single backticks.

[codecov]

Codecov Report

Merging #1122 into master will decrease coverage by <.01%.
The diff coverage is 98.79%.

@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
- Coverage   99.51%   99.51%   -0.01%     
==========================================
  Files         104      105       +1     
  Lines       12631    12711      +80     
  Branches      968      969       +1     
==========================================
+ Hits        12570    12649      +79     
- Misses         40       41       +1     
  Partials       21       21

Impacted Files	Coverage Δ
trio/from_thread.py	100% <100%> (ø)
trio/tests/test_threads.py	100% <100%> (ø)	⬆️
trio/__init__.py	97.5% <100%> (+0.13%)	⬆️
trio/_threads.py	99.12% <96.42%> (-0.88%)	⬇️

[codecov]

Codecov Report

Merging #1122 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
+ Coverage   99.54%   99.55%   +<.01%     
==========================================
  Files         104      105       +1     
  Lines       12646    12715      +69     
  Branches      967      969       +2     
==========================================
+ Hits        12589    12658      +69     
  Misses         36       36              
  Partials       21       21

Impacted Files	Coverage Δ
trio/_core/_exceptions.py	100% <ø> (ø)	⬆️
trio/from_thread.py	100% <100%> (ø)
trio/_threads.py	100% <100%> (ø)	⬆️
trio/tests/test_threads.py	100% <100%> (ø)	⬆️
trio/__init__.py	100% <100%> (ø)	⬆️

[epellis]

Okay, hello again! I have fixed up all of the issues mentioned in the last round of the PR and have also made sure all of the documentation and code coverage is at par with or better than where it was before.

[epellis]

@njsmith mind giving any feedback on this? I think I made all of the requested changes but want to confirm before I merge this.

[njsmith]

Sorry for the slow follow up here.

This is looking good overall, so now you get all the fiddly little comments to polish up the details :-)

[njsmith]

It looks like these double backticks still need to be replaced with single backticks.

[epellis]

@njsmith Got some more time to polish this up, can you let me know if the latest round of changes are up to spec? Thanks as always!

[njsmith]

@epellis Looks great, thanks! BTW, I think github is listing your last few commits as coming from your work account – doesn't matter to me, but FYI in case you wanted to keep those separate.

Any thoughts on what you want to work on next?

[epellis]

@njsmith Thanks for letting me know, I thought I signed out of that account on this laptop but I guess my git cache says otherwise 🤷‍♂. I'd love to continue working with the scheduler and threading/async interop so I'll take stock of how the rest of #1085 and friends are doing and try to comment there in the next week or so with chunks I think I could manage. Thanks again for your feedback, this PR was a really enjoyable process! :)