[issue34730] aclose() doesn't stop raise StopAsyncIteration / GeneratorExit to __anext__()

2020-08-20 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> Perhaps it's time to restart the original discussion, whether `aclose()` 
> should cancel pending `anext`.

I'm still not aware of any way to make this work, technically. So it's a moot 
point unless someone has a proposal.

--

___
Python tracker 
<https://bugs.python.org/issue34730>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41543] contextlib.nullcontext doesn't work with async context managers

2020-08-13 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It does seem pretty harmless in this case.

--

___
Python tracker 
<https://bugs.python.org/issue41543>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41303] perf_counter result does not count system sleep time in Mac OS

2020-08-04 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I made a record of my investigations here: 
https://github.com/python-trio/trio/issues/1586

One reason we might care about this is that asyncio ought to be using a clock 
that stops ticking while suspended.

(On which note: Please don't switch macOS to use 
clock_gettime(CLOCK_MONOTONIC); that would be a breaking change for us!)

At least Linux, macOS, FreeBSD, and Windows all have a way to access a 
monotonic clock that stops ticking while the system is suspended.

--

___
Python tracker 
<https://bugs.python.org/issue41303>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41229] Asynchronous generator memory leak

2020-07-19 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Oh! I see it. This is actually working as intended.

What's happening is that the event loop will clean up async generators when 
they're garbage collected... but, this requires that the event loop get a 
chance to run. In the demonstration program, the main task creates lots of 
async generator objects, but never returns to the main loop. So they're all 
queued up to be collected, but it can't actually happen until you perform a 
real async operation. For example, try adding 'await asyncio.sleep(1)` before 
the input() call so that the event loop has a chance to run, and you'll see 
that the objects are collected immediately.

So this is a bit tricky, but this is actually expected behavior, and falls 
under the general category of "don't block the event loop, it will break stuff".

--
resolution:  -> not a bug
stage:  -> resolved
status: open -> closed

___
Python tracker 
<https://bugs.python.org/issue41229>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41229] Asynchronous generator memory leak

2020-07-19 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

...On closer examination, it looks like that Trio PR has at least one test that 
checks that async generators are collected promptly after they stop being 
referenced, and that test passes:

https://github.com/python-trio/trio/pull/1564/files#diff-c79a78487c2f350ba99059813ea0c9f9R38

So, I have no idea what's going on here.

--

___
Python tracker 
<https://bugs.python.org/issue41229>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41229] Asynchronous generator memory leak

2020-07-19 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

Huh, this is very weird. I can confirm that the async generator objects aren't 
cleaned up until loop shutdown on asyncio.

On the trio main branch, we don't yet use the `set_asyncgen_hooks` mechanism, 
and the async generator objects are cleaned up immediately.

However, if I check out this PR that will add it: 
https://github.com/python-trio/trio/pull/1564

...then we see the same bug happening with Trio: all the async generators are 
kept around until loop shutdown.

Also, it doesn't seem to be a circular references issue – if I explicitly call 
`gc.collect()`, then the asyncgen destructors are still *not* called; only 
shutting down the loop does it.

This doesn't make any sense, because asyncio/trio only keep weak references to 
the async generator objects, so they should still be freed.

So maybe the `set_asyncgen_hooks` code introduces a reference leak on async 
generator objects, or something?

--

___
Python tracker 
<https://bugs.python.org/issue41229>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue22239] asyncio: nested event loop

2020-07-08 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> 90% of everything people are doing here are in the context of HTTP services.  
>  The problem of, "something.a now creates state that other tasks might see" 
> is not a real "problem" that is solved by using IO-only explicit context 
> switching.  This is because in a CRUD-style application, "something" is not 
> going to be a process-local yet thread-global object that had to be created 
> specifically for the application (there's things like the database connection 
> pool and some registries that the ORM uses, but those problems are taken care 
> of and aren't specific to one particular application).

Yeah, in classic HTTP CRUD services the concurrency is just a bunch of 
stateless handlers running simultaneously. This is about the simplest possible 
kind of concurrency. There are times when async is useful here, but to me the 
main motivation for async is for building applications with more complex 
concurrency, that currently just don't get written because of the lack of good 
frameworks. So that 90% number might be accurate for right now, but I'm not 
sure it's accurate for the future.

> In the realm of Python HTTP/CRUD applications, async is actually very popular 
> however it is in the form of gevent and sometimes eventlet monkeypatching, 
> often because people are using async web servers like gunicorn.

A critical difference between gevent-style async and newer frameworks like 
asyncio and trio is that the newer frameworks put cancellation support much 
more in the foreground. To me cancellation is the strongest argument for 
'await' syntax, so I'm not sure experience with gevent is representative.

I am a bit struck that you haven't mentioned cancellation handling at all in 
your replies. I can't emphasize enough how much cancellation requires care and 
attention throughout the whole ecosystem.

> w.r.t the issue of writing everything as async and then using the coroutine 
> primitives to convert to "sync" as means of maintaining both facades, I don't 
> think that covers the fact that most DBAPI drivers are sync only

I think I either disagree or am missing something :-). Certainly for both 
edgedb and urllib3, when they're running in sync mode, they end up using 
synchronous network APIs at the "bottom", and it works fine.

The greenlet approach does let you skip adding async/await annotations to your 
code, so it saves some work that way. IME this isn't particularly difficult 
(you probably don't need to change any logic at all, just add some extra 
annotations), and to me the benefits outweigh that, but I can see how you might 
prefer greenlets either temporarily as a transition hack or even in the long 
term.

--

___
Python tracker 
<https://bugs.python.org/issue22239>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue22239] asyncio: nested event loop

2020-07-07 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> Whether or not one buys that, the point of my approach is that SQLAlchemy 
> itself *will* publish async methods.  End user code *will not* ever context 
> switch to another task without them explicitly calling using an await.

Oh, I thought the primary problem for SQLAlchemy supporting async is that the 
ORM needs to do IO from inside __getattr__ methods. So I assumed that the 
reason you were so excited about greenlets was that it would let you use 
await_() from inside those __getattr__ calls, which would involve exposing your 
use of greenlets as part of your public API.

If you're just talking about using greenlets internally and then writing both 
sync and async shims to be your public API, then obviously that reduces the 
risks. Maybe greenlets will cause you problems, maybe not, but either way you 
know what you're getting into and the decision only affects you :-). But, if 
that's all you're using them for, then I'm not sure that they have a 
significant advantage over the edgedb-style synchronous wrapper or the 
unasync-style automatically generated sync code.

--

___
Python tracker 
<https://bugs.python.org/issue22239>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue22239] asyncio: nested event loop

2020-07-07 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Yeah, writing a trivial "event loop" to drive actually-synchronous code is 
easy. Try it out:

-

async def f():
print("hi from f()")
await g()

async def g():
print("hi from g()")

# This is our event loop:
coro = f()
try:
coro.send(None)
except StopIteration:
pass

-

I guess there's technically some overhead, but it's tiny.

I think dropping 'await' syntax has two major downsides:

Downside 1: 'await' shows you where context switches can happen: As we know, 
writing correct thread-safe code is mind-bendingly hard, because data can 
change underneath your feet at any moment. With async/await, things are much 
easier to reason about, because any span of code that doesn't contain an 
'await' is automatically atomic:

---
async def task1():
# These two assignments happen atomically, so it's impossible for
# another task to see 'someobj' in an inconsistent state.
someobj.a = 1
someobj.b = 2
---

This applies to all basic operations like __getitem__ and __setitem__, 
arithmetic, etc. -- in the async/await world, any combination of these is 
automatically atomic.

With greenlets OTOH, it becomes possible for another task to observe someobj.a 
== 1 without someobj.b == 2, in case someobj.__setattr__ internally invoked an 
await_(). Any operation can potentially invoke a context switch. So debugging 
greenlets code is roughly as hard as debugging full-on multithreaded code, and 
much harder than debugging async/await code.

This first downside has been widely discussed (e.g. Glyph's "unyielding" blog 
post), but I think the other downside is more important:

- 'await' shows where cancellation can happen: Synchronous libraries don't have 
a concept of cancellation. OTOH async libraries *are* expected to handle 
cancellation cleanly and correctly. This is *not* trivial. With your 
sqlalchemy+greenlets code, you've introduced probably hundreds of extra 
unwinding paths that you've never tested or probably even thought about. How 
confident are you that they all unwind correctly (e.g. without corrupting 
sqlalchemy's internal state)? How do you plan to even find them, given that you 
can't see the cancellation points in your code? How can your users tell which 
operations could raise a cancelled exception?

AFAICT you can't reasonably build systems that handle cancellation correctly 
without some explicit mechanism to track where the cancellation can happen. 
There's a ton of prior art here and you see this conclusion over and over.

tl;dr: I think switching from async/await -> greenlets would make it much 
easier to write programs that are 90% correct, and much harder to write 
programs that are 100% correct. That might be a good tradeoff in some 
situations, but it's a lot more complicated than it seems.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue22239>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue41151] Support for new Windows pseudoterminals in the subprocess module

2020-06-28 Thread Nathaniel Smith


New submission from Nathaniel Smith :

So Windows finally has pty support: 
https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/

However, the API is a bit weird. Unlike Unix, when you create a Windows pty, 
there's no way to directly get access to the "slave" handle. Instead, you first 
call CreatePseudoConsole to get a special "HPCON" object, which is similar to a 
Unix pty master. And then you can have to use a special CreateProcess 
incantation to spawn a child process that's attached to our pty.

Specifically, what you have to do is set a special entry in the 
"lpAttributeList", with type "PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE".

Details: 
https://docs.microsoft.com/en-us/windows/console/creating-a-pseudoconsole-session

Unfortunately, the subprocess module does not provide any way to pass arbitrary 
attributes through lpAttributeList, which means that it's currently impossible 
to use pty support on Windows without reimplementing the whole subprocess 
module.

It would be nice if the subprocess module could somehow support this.

Off the top of my head, I can think of three possible APIs:

Option 1: full support for Windows ptys: this would require wrapping 
CreatePseudoConsole, providing some object to represent a Windows pty, etc. 
This is fairly complex (especially since CreatePseudoConsole requires you to 
pass in some pipe handles, and the user might want to create those themselves).

Option 2: minimal support for Windows ptys: add another supported field to the 
subprocess module's lpAttributeList wrapper, that lets the user pass in an 
"HPCON" cast to a Python integer, and stuffs it into the attribute list. This 
would require users to do all the work to actually *get* the HPCON object, but 
at least it would make ptys possible to use.

Option 3: generic support for unrecognized lpAttributeList entries: add a field 
to the subprocess module's lpAttributeList wrapper that lets you add arbitrary 
entries, specified by type number + arbitrary pointer/chunk of bytes. (Similar 
to how Python's ioctl or setsockopt wrappers work.) Annoyingly, it doesn't seem 
to be enough to just pass in a buffer object, because for pseudoconsole 
support, you actually have to pass in an opaque "HPCON" object directly. (This 
is kind of weird, and might even be a bug, see: 
https://github.com/microsoft/terminal/issues/6705)

--
messages: 372526
nosy: giampaolo.rodola, gregory.p.smith, njs
priority: normal
severity: normal
status: open
title: Support for new Windows pseudoterminals in the subprocess module
type: enhancement
versions: Python 3.10

___
Python tracker 
<https://bugs.python.org/issue41151>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40916] Proposed tweak to allow for per-task async generator semantics

2020-06-08 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

FWIW, this seems like a pretty straightforward improvement to me.

--

___
Python tracker 
<https://bugs.python.org/issue40916>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue31254] WeakKeyDictionary/Mapping doesn't call __missing__

2020-06-06 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
pull_requests:  -19900

___
Python tracker 
<https://bugs.python.org/issue31254>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue31254] WeakKeyDictionary/Mapping doesn't call __missing__

2020-06-04 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Just found this while searching to see if we had an existing way to combine 
WeakValueDictionary + defaultdict. The use case I've run into a few times is 
where you need a keyed collection of mutexes, like e.g. asyncio.Lock objects. 
You want to make sure that if there are multiple tasks trying to access the 
same key/resource, they all acquire the same lock, but you don't want to use a 
regular defaultdict, because that will end up growing endlessly as different 
keys/resources are encountered.

It'd be very handy to do something like:

lock_dict = WeakValueDictionary(default=asyncio.Lock)

async with lock_dict[key]:
...

and have everything magically work.

The alternative is to open-code the default handling at the call site, either:

# Wasteful: creates a new Lock object on every usage,
# regardless of whether it's actually needed.
async with lock_dict.setdefault(key, asyncio.Lock()):
...

Or else the verbose:

lock = lock_dict.get(key)
if lock is None:
lock = lock_dict[key] = asyncio.Lock()
async with lock:
...

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue31254>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40789] C-level destructor in PySide2 breaks gen_send_ex, which assumes it's safe to call Py_DECREF with a live exception

2020-05-28 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Filed with PySide2: https://bugreports.qt.io/browse/PYSIDE-1313

--
resolution:  -> not a bug
stage:  -> resolved
status: open -> closed

___
Python tracker 
<https://bugs.python.org/issue40789>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40789] C-level destructor in PySide2 breaks gen_send_ex, which assumes it's safe to call Py_DECREF with a live exception

2020-05-28 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I don't think I understand what you mean by "reentrant"... we don't have any 
single piece of code that's calling back into itself. The question is about 
what the tp_dealloc contract is.

Digging into it more, it looks like the standard is for 
typeobject.c:slot_tp_finalize to save/restore exceptions when invoking 
Python-level __del__ methods, rather than it being the responsibility of the 
tp_dealloc or tp_finalizer caller. (And finding that code also answers my next 
question, which was going to be whether there are any other dance steps you 
have to do when re-entering CPython from a tp_dealloc!)

So I guess this is a PySide2 bug.

--

___
Python tracker 
<https://bugs.python.org/issue40789>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40789] C-level destructor in PySide2 breaks gen_send_ex, which assumes it's safe to call Py_DECREF with a live exception

2020-05-26 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Consider the following short program. demo() is a trivial async function that 
creates a QObject instance, connects a Python signal, and then exits. When we 
call `send(None)` on this object, we expect to get a StopIteration exception.

-

from PySide2 import QtCore

class MyQObject(QtCore.QObject):
sig = QtCore.Signal()

async def demo():
myqobject = MyQObject()
myqobject.sig.connect(lambda: None)
return 1

coro = demo()
try:
coro.send(None)
except StopIteration as exc:
print(f"OK: got {exc!r}")
except SystemError as exc:
print(f"WTF: got {exc!r}")

-

Actual output (tested on 3.8.2, but I think the code is present on all 
versions):

-
StopIteration: 1
WTF: got SystemError(" returned NULL 
without setting an error")
-

So there are two weird things here: the StopIteration exception is being 
printed on the console for some reason, and then the actual `send` method is 
raising SystemError instead of StopIteration.

Here's what I think is happening:

In genobject.c:gen_send_ex, when the coroutine finishes, we call 
_PyGen_SetStopIterationValue to raise the StopIteration exception:

https://github.com/python/cpython/blob/404b23b85b17c84e022779f31fc89cb0ed0d37e8/Objects/genobject.c#L241

Then, after that, gen_send_ex clears the frame object and drops references to 
it:

https://github.com/python/cpython/blob/404b23b85b17c84e022779f31fc89cb0ed0d37e8/Objects/genobject.c#L266-L273

At this point, the reference count for `myqobject` drops to zero, so its 
destructor is invoked. And this destructor ends up clearing the current 
exception again. Here's a stack trace:

-
#0  0x00677eb7 in _PyErr_Fetch (p_traceback=0x7ffd9fda77d0, 
p_value=0x7ffd9fda77d8, p_type=0x7ffd9fda77e0, tstate=0x2511280)
at ../Python/errors.c:399
#1  _PyErr_PrintEx (tstate=0x2511280, set_sys_last_vars=1) at 
../Python/pythonrun.c:670
#2  0x7f1afb455967 in 
PySide::GlobalReceiverV2::qt_metacall(QMetaObject::Call, int, void**) ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/PySide2/libpyside2.abi3.so.5.14
#3  0x7f1afaf2f657 in void doActivate(QObject*, int, void**) ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/PySide2/Qt/lib/libQt5Core.so.5
#4  0x7f1afaf2a37f in QObject::destroyed(QObject*) ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/PySide2/Qt/lib/libQt5Core.so.5
#5  0x7f1afaf2d742 in QObject::~QObject() ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/PySide2/Qt/lib/libQt5Core.so.5
#6  0x7f1afb852681 in QObjectWrapper::~QObjectWrapper() ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/PySide2/QtCore.abi3.so
#7  0x7f1afbf785bb in SbkDeallocWrapperCommon ()
   from 
/home/njs/.user-python3.8/lib/python3.8/site-packages/shiboken2/libshiboken2.abi3.so.5.14
#8  0x005a4fbc in subtype_dealloc (self=)
at ../Objects/typeobject.c:1289
#9  0x005e8c08 in _Py_Dealloc (op=) at 
../Objects/object.c:2215
#10 _Py_DECREF (filename=0x881795 "../Objects/frameobject.c", lineno=430, 
op=) at ../Include/object.h:478
#11 frame_dealloc (f=Frame 0x7f1afc572dd0, for file qget-min.py, line 12, in 
demo ())
at ../Objects/frameobject.c:430
#12 0x004fdf30 in _Py_Dealloc (
op=Frame 0x7f1afc572dd0, for file qget-min.py, line 12, in demo ())
at ../Objects/object.c:2215
#13 _Py_DECREF (filename=, lineno=279, 
op=Frame 0x7f1afc572dd0, for file qget-min.py, line 12, in demo ())
at ../Include/object.h:478
#14 gen_send_ex (gen=0x7f1afbd08440, arg=, exc=, 
closing=) at ../Objects/genobject.c:279

--

We can read the source for PySide::GlobalReceiverV2::qt_metacall here: 
https://sources.debian.org/src/pyside2/5.13.2-3/sources/pyside2/libpyside/globalreceiverv2.cpp/?hl=310#L310

And we see that it (potentially) runs some arbitrary Python code, and then 
handles any exceptions by doing:

if (PyErr_Occurred()) {
PyErr_Print();
}

This is intended to catch exceptions caused by the code it just executed, but 
in this case, gen_send_ex ends up invoking it with an exception already active, 
so PySide2 gets confused and clears the StopIteration.

---

OK so... what to do. I'm actually not 100% certain whether this is a CPython 
bug or a PySide2 bug.

In PySide2, it could be worked around by saving the exception state before 
executing that code, and then restoring it afterwards.

In gen_send_ex, it could be worked around by dropping the reference to the 
frame before setting the StopIteration exception.

In CPython in general, it could be worked around by not invoking deallocators 
with a live exception... I'm actually pretty surprised that this is even 
possible! It seems like having a live exception when you start executing 
arbitrary Python code would be bad. So maybe that's the real bug? Adding both 
"asyncio" a

[issue39148] DatagramProtocol + IPv6 does not work with ProactorEventLoop

2020-05-18 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I think I fixed the buildbot issues in GH-20170, but I can't seem to reach the 
buildbot site right now, so it's hard to know for sure!

--
versions:  -Python 3.6

___
Python tracker 
<https://bugs.python.org/issue39148>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39148] DatagramProtocol + IPv6 does not work with ProactorEventLoop

2020-05-18 Thread Nathaniel Smith


Nathaniel Smith  added the comment:


New changeset 58205a0217e91232cc1e945dbfe4e387d636eb76 by Nathaniel J. Smith in 
branch 'master':
bpo-39148: fixup to account for IPV6_ENABLED being moved (GH-20170)
https://github.com/python/cpython/commit/58205a0217e91232cc1e945dbfe4e387d636eb76


--

___
Python tracker 
<https://bugs.python.org/issue39148>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39148] DatagramProtocol + IPv6 does not work with ProactorEventLoop

2020-05-18 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
pull_requests: +19469
pull_request: https://github.com/python/cpython/pull/20170

___
Python tracker 
<https://bugs.python.org/issue39148>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40607] asyncio.wait_for should reraise future exception even if timeout expires

2020-05-12 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

makes sense to me

On Tue, May 12, 2020 at 10:14 PM Chris Jerdonek  wrote:
>
>
> Chris Jerdonek  added the comment:
>
> Also adding Nathaniel since he's the one that filed #32751. Nathaniel, do you 
> agree that if an exception occurs while waiting for the cancellation, the 
> exception should be what's raised instead of TimeoutError?
>
> --
> nosy: +chris.jerdonek, njs
>
> ___
> Python tracker 
> <https://bugs.python.org/issue40607>
> ___

--

___
Python tracker 
<https://bugs.python.org/issue40607>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38323] asyncio: MultiLoopWatcher has a race condition (test_asyncio: test_close_kill_running() hangs on AMD64 RHEL7 Refleaks 3.x)

2020-05-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

How do you know that your reproducer is showing the same bug, or anything
related to signals? IIUC subprocess waiting by default doesn't involve a
signal handler.

On Sat, May 9, 2020, 14:40 Chris Jerdonek  wrote:

>
> Chris Jerdonek  added the comment:
>
> > this seems like an awful lot of energy to spend on some code
> that's not even used by default.
>
> True, but it seems likely to me that this signals :) a deeper, more
> general issue about CPython / signals (which is why I spent time on it).
> For example, my reproducer script doesn't use MultiLoopWatcher. I'd like to
> see if it can be reproduced with signals other than SIGCHLD, too.
>
> --
>
> ___
> Python tracker 
> <https://bugs.python.org/issue38323>
> ___
>

--

___
Python tracker 
<https://bugs.python.org/issue38323>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38323] asyncio: MultiLoopWatcher has a race condition (test_asyncio: test_close_kill_running() hangs on AMD64 RHEL7 Refleaks 3.x)

2020-05-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I don't have any particular insight into the bug, but I do have an
observation: this seems like an awful lot of energy to spend on some code
that's not even used by default. Would it make sense to deprecate it and
stick with ThreadedChildWatcher?

On Sat, May 9, 2020, 05:39 Chris Jerdonek  wrote:

>
> Chris Jerdonek  added the comment:
>
> I'm adding Nathaniel in case he can recognize this as one of the signal
> handler cases he's already come across.
>
> --
> nosy: +njs
>
> ___
> Python tracker 
> <https://bugs.python.org/issue38323>
> ___
>

--

___
Python tracker 
<https://bugs.python.org/issue38323>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue40213] contextlib.aclosing()

2020-04-06 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
nosy: +ncoghlan, yselivanov

___
Python tracker 
<https://bugs.python.org/issue40213>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37373] Configuration of windows event loop for libraries

2020-02-25 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> was Tornado the only project experiencing this pain

At least twisted and anyio are still broken on 3.8+Windows because of this 
change:

https://twistedmatrix.com/trac/ticket/9766
https://github.com/agronholm/anyio/issues/77

--

___
Python tracker 
<https://bugs.python.org/issue37373>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39606] Regression: it should be possible to close async iterators multiple times

2020-02-13 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 
<https://bugs.python.org/issue39606>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39606] Regression: it should be possible to close async iterators multiple times

2020-02-13 Thread Nathaniel Smith


Nathaniel Smith  added the comment:


New changeset f464edf3239f7867fe31c9cd238a68fb3b90feaa by Nathaniel J. Smith in 
branch '3.7':
bpo-39606: allow closing async generators that are already closed (GH-18475) 
(GH-18502)
https://github.com/python/cpython/commit/f464edf3239f7867fe31c9cd238a68fb3b90feaa


--

___
Python tracker 
<https://bugs.python.org/issue39606>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39386] Prevent double awaiting of async iterator

2020-02-13 Thread Nathaniel Smith


Nathaniel Smith  added the comment:


New changeset f464edf3239f7867fe31c9cd238a68fb3b90feaa by Nathaniel J. Smith in 
branch '3.7':
bpo-39606: allow closing async generators that are already closed (GH-18475) 
(GH-18502)
https://github.com/python/cpython/commit/f464edf3239f7867fe31c9cd238a68fb3b90feaa


--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue39386>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39386] Prevent double awaiting of async iterator

2020-02-13 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
pull_requests: +17878
pull_request: https://github.com/python/cpython/pull/18502

___
Python tracker 
<https://bugs.python.org/issue39386>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39606] Regression: it should be possible to close async iterators multiple times

2020-02-13 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
pull_requests: +17877
pull_request: https://github.com/python/cpython/pull/18502

___
Python tracker 
<https://bugs.python.org/issue39606>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39386] Prevent double awaiting of async iterator

2020-02-12 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
pull_requests: +17844
pull_request: https://github.com/python/cpython/pull/18475

___
Python tracker 
<https://bugs.python.org/issue39386>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39606] Regression: it should be possible to close async iterators multiple times

2020-02-12 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
keywords: +patch
pull_requests: +17843
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/18475

___
Python tracker 
<https://bugs.python.org/issue39606>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39606] Regression: it should be possible to close async iterators multiple times

2020-02-10 Thread Nathaniel Smith


New submission from Nathaniel Smith :

In bpo-39386, the 'aclose' method for async generators was fixed so that the 
following broken code would correctly raise an error:

# -- bad code --
async def agen_fn():
yield

async def do_bad_thing():
agen = agen_fn()
aclose_coro = agen.aclose()
await aclose_coro
# Should raise RuntimeError:
await aclose_coro

asyncio.run(do_bad_thing())

# -

However, the merged fix also broke the following correct code, which should 
*not* raise an error:

# -- good code --
async def agen_fn():
yield

async def do_good_thing():
agen = agen_fn()
await agen.aclose()
# Should *not* raise an error, but currently does in Python dev branches:
await agen.aclose()

asyncio.run(do_good_thing())

# 

It's not supported to iterate the same coroutine object twice. However, making 
two *independent* calls to aclose should return two independent coroutine 
objects, and it should be legal to iterate over each object once.

This can also occur even if there's only a single call to 'aclose'. For 
example, this is the recommended idiom for making sure that an async generator 
correctly closes all its resources:

# -- good code --
async def agen_fn():
yield

async def careful_loop():
agen = agen_fn()
try:
async for _ in agen:
pass
finally:
await agen.aclose()

asyncio.run(careful_loop())

# ---

On released Python, the code above runs correctly. On the latest Python dev 
branches, it raises a RuntimeError.

So basically the problem is that the fix for bpo-39386 is storing the "was 
aclose iterated?" state on the async generator object, but in fact it needs to 
be stored on the aclose coroutine object.

--
keywords: 3.7regression, 3.8regression, 3.9regression
messages: 361783
nosy: asvetlov, lukasz.langa, ned.deily, njs, yselivanov
priority: release blocker
severity: normal
status: open
title: Regression: it should be possible to close async iterators multiple times
versions: Python 3.7, Python 3.8, Python 3.9

___
Python tracker 
<https://bugs.python.org/issue39606>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue18233] SSLSocket.getpeercertchain()

2020-01-31 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I'm not sure I agree about assuming that users will be able to work around 
these issues... I mean, nothing personal, I'm sure you're well-informed and 
maybe your code would work fine, but if you don't understand my example then 
how can you be entirely confident that you really understand all the risks that 
your code needs to watch out for?

Anyway, the proposed PR exposes both methods, so that's not a problem; it's 
just waiting on someone with openssl expertise like Christian to review it.

--

___
Python tracker 
<https://bugs.python.org/issue18233>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue18233] SSLSocket.getpeercertchain()

2020-01-30 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> For the concern issue, as I understand it, the ability to call getpeercert() 
> or the proposed getpeercertchain() is only after the TLS session has been 
> established.  As such, the SSL socket already established that there exists a 
> valid chain of trust.  Thus these methods are primarily to provide visibility 
> into what the peer passed *after* it had been authenticated, right?

The issue is that "the cert chain provided by the client" and "the cert chain 
that was validated as trustworthy" might be different. For example, say you 
have trust roots A and B, and I have a leaf certificate with a valid signature 
by B. If I pass the chain [A, leaf cert], then openssl's trust validation code 
might look at that and say "yep, this leaf cert is signed by root B, I trust 
root B, cool, the connection is good. Also there's a random A cert here, but 
whatever, that doesn't change anything".

In this case, for your use case, you want to get back the chain [B, leaf cert], 
because that's the chain that was actually validated. If you instead got the 
chain the client gave you, then you might be fooled into thinking that this 
cert chained to root A, when it actually didn't, and make the wrong trust 
decision.

That's why we need to be careful to distinguish between these two possible 
chains.

--

___
Python tracker 
<https://bugs.python.org/issue18233>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue39148] DatagramProtocol + IPv6 does not work with ProactorEventLoop

2019-12-28 Thread Nathaniel Smith


Change by Nathaniel Smith :


--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue39148>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-12-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> revalidate pid licenses

It means autocorrect mangled the text... I don't remember what word that's 
supposed to be, but basically I meant "revalidate the pid hasn't been reaped"

> Wouldn't it be sufficient to add
> 
> if self.returncode is not None:
> return self.returncode
> 
> at the top of poll()?

No, the race condition is:

1. Process exits
[some time passes]
2. Thread 1 and Thread 2 both call poll() simultaneously
3. Thread 1 and Thread 2 both see that the process hasn't been reaped
4. Thread 1 attempts to take the blocking_wait_lock, to make sure no-one is 
blocked in wait(). It succeeds.
5. Thread 2 attempts to take the blocking_wait_lock, and it fails (because 
Thread 1 already has it). Thread 2's poll() returns saying that the process 
hasn't exited yet (which is wrong!)
6. Thread 1 calls waitpid(..., WNOHANG) and reaps the process, then releases 
the lock.

So adding a check at the top (= step 3) doesn't help. The problem is in steps 
4/5.

--

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38699] socket: change listen() default backlog from 128 to 4096?

2019-12-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Yeah, I don't think it matters that much. There are lots of random gotchas that 
you have to watch out for using the socket module.

To be clear: I do still think that using a large value by default, and clamping 
user input values at 65535, would be improvements. I just don't think they're 
important enough to spend energy arguing about it :-).

The reason I'm not worried about large backlogs wasting kernel memory is that 
servers all call accept as fast as they can. This means any incoming connection 
flood will end up taking memory regardless of the size of the kernel-side 
buffer. And I'm pretty sure the kernel buffer ia a linked list, so setting it 
to a large value doesn't cost anything if you're not using it.

Anyway asyncio at least should probably update its default.

--

___
Python tracker 
<https://bugs.python.org/issue38699>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-12-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

>  Thread B thinks the process is still running, so it calls waitid+WNOHANG on 
> a stale PID, with unpredictable results.

I'm pretty sure you mean WNOWAIT, right? The actual reaping step (which might 
use WNOHANG) is already protected by a lock, but there's a danger that a 
process might try to perform the blocking-until-exit step (which uses WNOWAIT) 
on a stale pid.


Good catch!

I think this can be fixed by using a second lock around all of .wait(). But 
this is a bit tricky because we also need to account for concurrent .poll() 
calls. So something like:

def wait(self):
# Don't take the big lock unless the process is actually still running
# This isn't just an optimization; it's critical for poll() correctness
if self.poll() is not None:
return self.returncode

with self._blocking_wait_lock:
with self._returncode_lock:
revalidate pid licenses

block_until_child_exits()

with self._returncode_lock:
reap the child and mark it as reaped

def poll(self):
# if someone is already blocked waiting for the child, then it definitely
# hasn't exited yet, so we don't need to call waitpid ourselves.
# This isn't just an optimization; it's critical for correctness.
if not self._blocking_wait_lock.acquire(blocking=False):
return None
try:
with self._returncode_lock:
do the actual poll + returncode updating
finally:
self._blocking_wait_lock.release()

Notes:

If there's already a wait() running, and someone calls poll(), then we have to 
make sure the poll() can't reap the process out from under the wait(). To fix 
that, poll skips trying in case wait is already running.

But, this could introduce its own race condition: if a process has exited but 
we haven't noticed yet, and then someone calls wait() and poll() 
simultaneously, then the wait() might take the lock, then poll() notices the 
lock is taken, and concludes that the process can't have exited yet. If course 
wait() will immediately reap the process and drop the lock, but by this point 
poll() has already returned the wrong information.

The problem is that poll() needs to be able to treat "the blocking wait lock is 
taken" as implying "the process hasn't exited yet". So to make that implication 
true, we add a check at the top of wait().

Of course if a process exits while wait() is running, and someone calls poll() 
in that brief interval between it exiting and wait() noticing, then poll() 
could again fail to report it exiting. But I think this is fine: it's ok if 
poll() is a fraction of a second out of date; that race condition is inherent 
in its API. The problem would be if the fails to notice a process that exited a 
while ago and is just sitting around waiting to be reaped.

... Ugh but there is still one more race condition here. I think this fixes all 
the cases involving send_signal, wait, and poll interactions with each other, 
BUT we broke the case of two threads calling poll() at the same time. One 
thread will take the blocking_wait_lock, then the other will take this as 
evidence that someone is blocked in wait() and exit early, which isn't 
appropriate.

I think we can patch up this last race condition by adding yet one more lock: a 
simple

with self._poll_lock:

around the whole body of poll(), so that only one thread can call it at a time.

--

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38699] socket: change listen() default backlog from 128 to 4096?

2019-12-06 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Another subtlety that that code handles, and that the stdlib socket module 
might also want to handle: if the user passes in a custom backlog argument 
that's >65535, then we silently replace it with 66535 before passing it to the 
system. The reason is that many systems will silently truncate this value to 16 
bits, so if a user explicitly passes in 65536, what they get is a backlog of 1, 
which is probably not what they were hoping for.

--

___
Python tracker 
<https://bugs.python.org/issue38699>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38699] socket: change listen() default backlog from 128 to 4096?

2019-12-06 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Trio uses 65535 as the default backlog argument. There's a big comment here 
explaining why: 
https://github.com/python-trio/trio/blob/master/trio/_highlevel_open_tcp_listeners.py

This doesn't actually set the backlog to 65635; instead it tells the kernel to 
use the biggest backlog that it feels comfortable with (e.g. 
/proc/sys/net/core/somaxconn on Linux).

--

___
Python tracker 
<https://bugs.python.org/issue38699>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue18233] SSLSocket.getpeercertchain()

2019-11-26 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

There's another important use case for this, that hasn't been discussed here. 
If you want to use openssl for TLS + the system trust store to verify 
certificates, then you need to disable openssl's certificate validation, 
perform the handshake, and then extract the certificate chain that there peer 
sent and pass it to the system native APIs to validate.

For this case, we don't need to do any validation or resolution on the chain – 
we just want to pull out the DER that the peer sent. AFAICT, the lack of this 
functionality is the one major blocker to using the system trust store with the 
'ssl' module.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue18233>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37228] UDP sockets created by create_datagram_endpoint() allow by default multiple processes to bind the same port

2019-11-21 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I was assuming we'd only do this on Linux, since that's where the bug is... 
though now that you mention it the Windows behavior is probably wonky too.

SO_REUSEADDR and SO_REUSEPORT have different semantics on all three of 
Windows/BSD/Linux. A higher-level interface like asyncio doesn't necessarily 
want to map its kwargs directly to non-portable low-level options like this.

--

___
Python tracker 
<https://bugs.python.org/issue37228>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38692] add a pidfd child process watcher

2019-11-20 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I don't know about podman, but it sounds like mock and docker both use buggy 
sandboxing: https://bugzilla.redhat.com/show_bug.cgi?id=1770154

--

___
Python tracker 
<https://bugs.python.org/issue38692>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37228] UDP sockets created by create_datagram_endpoint() allow by default multiple processes to bind the same port

2019-11-20 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> Now your updated docs and warning read more like we are working around a 
> Linux security bug which is not really the case - this behavior was 
> intentionally added to the kernels and some of the code I do for a living 
> relies on it to work properly. Admittedly the restriction of having the same 
> UID wouldn't hurt.

I think you can use SO_REUSEPORT instead, and for UDP sockets it's identical to 
SO_REUSEADDR except with the same-UID restriction added?

If that's right then it might make sense to unconditionally switch SO_REUSEADDR 
-> SO_REUSEPORT, even in existing Python releases – on the theory that it fixes 
the main security hole, while being back-compatible enough to be acceptable for 
a point release.

--

___
Python tracker 
<https://bugs.python.org/issue37228>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37228] UDP sockets created by create_datagram_endpoint() allow by default multiple processes to bind the same port

2019-11-18 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Ouch, that's nasty. It also has security implications. For example, suppose you 
have a multi-user computer, where one user is has a video call going, which 
transfers packets over UDP. Another user could check what port they're using, 
bind to the same port, and steal half the packets, letting them effectively 
listen in on the first user's phone call.

Well, hopefully most conferencing tools use end-to-end encryption. But the 
point is that we don't normally let unprivileged users steal network traffic 
intended for other users.

--

___
Python tracker 
<https://bugs.python.org/issue37228>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38764] Deterministic globbing.

2019-11-10 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> I saw the article as well, but think auto-sorting would have just provided a 
> thin mask over their serious data pipeline bugs.

This seems like an inappropriately elitist attitude. I'm sure their code has 
bugs; so does mine and yours. But in fact they did test their code thoroughly, 
demonstrated that it produced correct results on their system, and it would 
have produced correct results for their downstream users too if Python's 'glob' 
had behaved consistently across platforms. Python isn't just for folks who can 
"git gud" to meet your arbitrary standards.

In addition, auto-sorting has a number of ergonomic benefits, beyond this one 
case: it makes output more deterministic, makes it easy to estimate progress 
given just a list of filenames being processed ("hmm, this script has been 
running for ten minutes and it's only on the Cs, I guess this will take a few 
hours"), and so on.

> Also, this isn't the only pathway to seeing file lists:  os.listdir, fnmatch, 
> etc.

os.listdir is a genuinely low-level OS-level tool, as indicated by the name, 
versus 'glob' which is designed as a convenient interface for high-level 
scripting.

fnmatch doesn't deal with either files or lists, so I'm not sure why you're 
bringing it up here.

> The reasons for the previous rejections still apply.

The reasons I see in the previous issues are:

- Sorting adds overhead
- It's hard to implement, esp. if iglob uses scandir internally
- iglob can't sort, so glob should be consistent
- it's easy to sort after the fact

But these are all false.

- The overhead added by sorting is negligible. In some informal benchmarks I 
measured it as ~4% extra CPU time in the worst case, if you're (a) running on 
Linux with it's ultra-optimized VFS caching, (b) all the directories are 
already cached in RAM, (c) you're not actually doing anything with the files 
except calling glob() and then discarding the result. Of course, it's always 
possible my benchmarks missed an important case; if you have benchmarks that 
show otherwise please post them so we can discuss.

- The implementation is trivial, as shown from the PR – literally just 
replacing two calls to 'list' with calls to 'sorted'.

- iglob being incremental doesn't actually pose any obstacles to sorting. Even 
if it's using scandir(), it still has to load each individual directory list 
into memory in one batch, to avoid the risk of leaking file descriptors.

- It's actually not trivial to sort filenames in the natural order afterwards, 
because you have to split each filename into components. It's also 
asymptotically slower than doing the sort as-you-go, O(total-files log 
total-files), versus O(total-files log maximum-individual-directory).

Given that there's at least one extremely clear case where not-sorting caused 
substantial harm, that there are lots of other minor cases where it's a nice 
win, and that the costs are negligible, sorting seems like the obviously 
correct default.

--

___
Python tracker 
<https://bugs.python.org/issue38764>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38712] add signal.pidfd_send_signal

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I guess a bikeshed question is whether it should be `os.pidfd_send_signal` 
(like `os.kill`) or `signal.pidfd_send_signal` (like `signal.pthread_kill`).

I don't actually care either way myself :-)

--

___
Python tracker 
<https://bugs.python.org/issue38712>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Hmm, you know, on further thought, it is actually possible to close this race 
for real, without pidfd.

What you do is split 'wait' into two parts: first it waits for me process to 
become reapable without actually reaping it. On Linux you can do this with 
waitid+WNOWAIT. On kqueue platforms you can do it with kqueue.

Then, you take a lock, and use it to atomically call waitpid/waitid to reap the 
process + update self.returncode to record that you've done so.

In send_signal, we use the same lock to atomically check whether the process 
has been reaped, and then send the signal if it hasn't.

That doesn't fix your test, but it does fix the race condition: as long as 
users only interact with the process through the Popen API, it guarantees that 
send_signal will never rather the wrong process.

--

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

You can't solve a time-of-check-to-time-of-use race by adding another check. I 
guess your patch might narrow the race window slightly, but it was a tiny 
window before and it's a tiny window after, so I don't really get it.

The test doesn't demonstrate a race condition. What it demonstrates is 
increased robustness against user code that corrupts Popen's internal state by 
reaping one of its processes behind its back.

That kind of robustness might be a good motivation on its own! (I actually just 
did a bunch of work to make trio more robust against user code closing fds 
behind its back, which is a similar kind of thing.) But it's a very different 
motivation than what you say in this bug.

--

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

But then deeper in the thread it sounded like you concluded that this didn't 
actually help anything, which is what I would expect :-).

--

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38630] subprocess.Popen.send_signal() should poll the process

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It sounds like there's actually nothing to do here? (Except maybe eventually 
switch to pidfd or similar, but Victor says he wants to use a different issue 
for that.) Can this be closed?

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue38630>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38692] add a pidfd child process watcher

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

hasattr is useful for supporting old versions of the os module, but asyncio 
doesn't have to care about that. You should probably try calling pidfd_open and 
see whether you get -ESYSCALL, and also maybe try passing it to poll(), since I 
think there might have been a release where the syscall existed but it wasn't 
pollable. Not sure what the error for that looks like.

--

___
Python tracker 
<https://bugs.python.org/issue38692>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38692] add a pidfd child process watcher

2019-11-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Thanks for the CC.

It would be nice to get `pidfd_send_signal` as well, while we're at it. But 
that could be a separate PR.

AFAICT the other bits of the pidfd API right now are some extra flags to clone, 
and an extra flag to waitid. The waitid flag is nice-to-have but not really 
urgent, since it's easy enough to use a flag even if the stdlib doesn't 
explicitly expose it. The clone flags are super interesting, but before we can 
use them, we need to have some way to access clone itself, and that's a big 
complicated project, so we definitely shouldn't worry about it here.

--

___
Python tracker 
<https://bugs.python.org/issue38692>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37373] Configuration of windows event loop for libraries

2019-11-03 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Yeah, that's the question. I've dug into the AFD_POLL stuff more now, and... 
There's a lot of edge cases to think about. It certainly *can* be done, because 
this is the primitive that 'select' and friends are built on, so obviously it 
can do anything they can do. But it also has some very sharp corners that 
'select' knows how to work around, and we might not, since the folks writing 
'select' could look at the internals and we can't.

The good news is that libuv has been shipping a version of this code for many 
years, and trio started shipping a version yesterday, so we can treat those as 
field experiments to gather data for asyncio. (I think that rust's mio library 
is also moving this way, but I'm not as familiar with the details. And wepoll 
is a good reference too, but I don't know how widely-deployed it is.)

The libuv implementation is very conservative, and falls back to calling 
'select' in a thread if anything looks the slightest bit weird. The author of 
that code told me that he now thinks that's too conservative, esp. since some 
if the issues they were worrying about in the win xp era are no longer 
relevant. So Trio's version is more aggressive. I'm very curious to see how it 
goes.

I do think the incompatibilities between the different aio event loops are 
really a problem and the ultimate goal should be to support the full feature 
set everywhere. The question is how to make that viable.

Getting more experience with AFD_POLL will help make it possible for aio to 
implement its own version, if that's the direction we want to go.

Maybe it would also be helpful to try to find the right folks inside Microsoft 
to get more information about this? My understanding is that their official 
position on AFD_POLL is "don't do that", but you can't put the genie back into 
the bottle...

Alternatively: it seems like this is really highlighting the downsides of aio 
maintaining its own written-from-scratch event loops. Would it make sense to 
promote uvloop to the One True Event Loop? I get that there are significant 
complications to doing that, but there are also significant benefits: we'd get 
a more mature event loop core that we don't have to maintain alone, and it 
would fix tornado.

--

___
Python tracker 
<https://bugs.python.org/issue37373>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38559] async generators aclose() behavior in 3.8

2019-10-22 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

I think conceptually, cancelling all tasks and waiting for them to exit is the 
right thing to do. That way you run as much shutdown code as possible in 
regular context, and also avoid this problem – if there are no tasks, then 
there can't be any tasks blocked in __anext__/asend/athrow/aclose, so calling 
aclose is safe.

This is also the right way to handle it from Trio's perspective – we always 
wait for all tasks to exit before exiting our run loop. For asyncio it might be 
a bit more complicated though so we should think through the trade-offs.

> The only way (at least known to me) that that could be the case if some tasks 
> ignores cancellation.  

Looking at runners.py, it seems like asyncio.run doesn't just call 
task.cancel(), but actually waits for all tasks to exit. (See the call to 
gather() inside _cancel_all_tasks.) So I think that does guarantee that by the 
time we hit shutdown_asyncgens(), we can be certain that all tasks have exited. 
(If a task ignores cancellation, then that will cause _cancel_all_tasks to 
hang, so we never reach shutdown_asyncgens at all. Not ideal, but not really 
our problem either – if someone complains we just tell them they need to stop 
ignoring cancellation if they want  their program to shutdown cleanly.)

So I think asyncio.run is actually totally fine with the 3.8.0 behavior. It's 
only explicit calls to shutdown_asyncgens that might run into this, and I think 
that's probably OK? If you're calling shutdown_asyncgens by hand then you're 
kind of taking responsibility for dealing with any subtle issues around 
shutdown.

And if we did allow aclose() to run at any time, then I worry that that could 
cause serious problems. Users can call aclose() at will, and it's generally 
good practice to always call aclose() on your async generators explicitly. So 
it's entirely possible that users will accidentally call aclose() themselves 
while they have another task is blocked in __anext__. And in that case what 
do we do?

The aclose call has to consume the generator's frame, by definition. But in the 
mean time, we still have this __anext__ coroutine object hanging around 
pointing to the consumed frame, that the task scheduler thinks needs to be 
resumed at some point in response to some event, and that can cause all kinds 
of weird situations. The __anext__ might try to resume while the aclose() is 
running. The __anext__ might get stuck and never resume, because of the code 
run by aclose() accidentally unregistering the event that was going to wake it 
up, so you just get a deadlock instead of a useful error. It might cause some 
other code entirely to crash – like if the __anext__ was blocked inside a call 
to 'await queue.get()', then aclose() interrupting that might corrupt the 
queue's internal state, so the next time some other task calls queue.put() they 
get an exception or other incorrect behavior. It all seems really tricky and 
messy to me.

So... this is super subtle and confusing, but I *think* the conclusion is that 
yeah, 3.8.0 is fine and there's no urgent action needed.

--

___
Python tracker 
<https://bugs.python.org/issue38559>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32561] Add API to io objects for cache-only reads/writes

2019-10-10 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> If you wanted to keep async disk access separate from the io module, then 
> what we'd have to do is to create a fork of all the code in the io module, 
> and add this feature to it.

Thinking about this again today, I realized there *might* be another option.

The tricky thing about supporting async file I/O is that users want the whole 
io module interface, and we don't want to have to reimplement all the 
functionality in TextIOWrapper, BufferedReader, BufferedWriter, etc. And we 
still need the blocking functionality too, for when we fall back to threads.

But, here's a possible hack. We could implement our own version of 'FileIO' 
that wraps around a real FileIO. Every operation just delegates to the 
underlying FileIO – but with a twist. Something like:

def wrapped_op(self, *args):
if self._cached_op.key == (op, args):
return self._cached_op.result
if MAGIC_THREAD_LOCAL.io_is_forbidden:
def cache_filler():
MAGIC_THREAD_LOCAL.io_is_forbidden = False
self._cached_op = self._real_file.op(*args)
raise IOForbiddenError(cache_filler)
return self._real_file.op(*args)

And then in order to implement an async operation, we do something like:

async def op(self, *args):
while True:
try:
# First try fulfilling the operation from cache
MAGIC_THREAD_LOCAL.io_is_forbidden = True
return self._io_obj.op(*args)
except IOForbiddenError as exc:
# We have to actually hit the disk
# Run the real IO operation in a thread, then try again
await in_thread(cache_filler)
finally:
del MAGIC_THREAD_LOCAL.io_is_forbidden

This is pretty convoluted: we keep trying the operation on the outer "buffered" 
object, seeing which low-level I/O operation it gets stuck on, doing that I/O 
operation, and trying again. There's all kinds of tricky non-local state here; 
like for example, there isn't any formal guarantee that the next time we try 
the "outer" I/O operation it will end up making exactly the same request to the 
"inner" RawIO object. If you try performing I/O operations on the same file 
from multiple tasks concurrently then you'll get all kinds of havoc. But if it 
works, then it does have two advantages:

First, it doesn't require changes to the io module, which is at least nice for 
experimentation.

And second, it's potentially compatible with the io_uring style of async disk 
I/O API. I don't actually know if this matters; if you look at the io_uring 
docs, the only reason they say they're more efficient than a thread pool is 
that they can do the equivalent of preadv(RWF_NOWAIT), and it's a lot easier to 
add preadv(RWF_NOWAIT) to a thread pool than it is to implement io_uring. But 
still, this approach is potentially more flexible than my original idea.

We'd still have to reimplement open() in order to set up our weird custom IO 
stacks, but hopefully that's not *too* bad, since it's mostly just a bunch of 
if statements to decide which wrappers to stick around the raw IO object.

My big concern is, that I'm not actually sure if this works :-).

The thing is, for this to work, we need 
TextIOWrapper/BufferedReader/BufferedWriter to be very well-behaved when the 
underlying operation raises an exception. In particular, if they're doing a 
complex operation that requires multiple calls to the underlying object, and 
the second call raises an exception, they need to keep the first call's results 
in their buffer so that next time they can pick up where they left off. And I 
have no idea if that's true.

I guess if you squint this is kind of like the non-blocking support in the io 
module – IOForbiddenError is like NonBlockingError. The big difference is that 
here, we don't have any "partial success" state at the low-level; either we do 
the operation immediately, or we punt and do the operation in a thread. Either 
way it completes as a single indivisible unit. So that might simplify things? 
From a quick skim of issue13322 it sounds like a lot of the problems with the 
current "non-blocking" mode come from these partial success states, but I 
haven't read it in detail.

--
title: Add API to io objects for non-blocking reads/writes -> Add API to io 
objects for cache-only reads/writes

___
Python tracker 
<https://bugs.python.org/issue32561>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32561] Add API to io objects for non-blocking reads/writes

2019-10-10 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

The proposal is to be able to run io module operations in two modes: the 
regular one, and one where performing actual I/O is forbidden – so if they go 
down the stack and can fulfill the operation from some in-memory buffer, great, 
they do that, and if not, they raise an error.

It turns out that this is actually the right primitive to enable async disk 
access. That's the only target use case, and there's no IO loop involved. If 
you wanted to keep async disk access separate from the io module, then what 
we'd have to do is to create a fork of all the code in the io module, and add 
this feature to it. Which doesn't seem like a good design :-).

--

___
Python tracker 
<https://bugs.python.org/issue32561>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37373] Configuration of windows event loop for libraries

2019-10-03 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It's also possible to implement 'select' on top of IOCP using undocumented 
private APIs. That's what libuv does (and how 'select' works internally). 
There's a bunch of information on this collected here: 
https://github.com/python-trio/trio/issues/52

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue37373>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38242] Revert the new asyncio Streams API

2019-09-23 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> I hope that the Trio project can minimize the number of methods they want to 
> add to those ABCs so that we don't need to duplicate a lot of functionality 
> in asyncio.Stream.  E.g. in the new asyncio.Stream there's a Stream.write() 
> coroutine; in Trio it's Stream.send_all().  I'm not entirely convinced that 
> "send_all()" is a good name, for example, even though I now understand the 
> motivation.  We can discuss that later in a relevant issue though.

Yeah, we definitely need to bikeshed the method names but we should do that 
separately. The actual methods are mostly settled though, and designed to be as 
minimal as possible. The complete set is:

- send data
- send EOF
- receive data
- close
- and *maybe* a kind of 'drain' method, which is quasi-optional and I'm hoping 
we can get rid of it; it's related to some latency/flow control/buffering 
issues that are too complex to discuss here

And we'll provide standard implementations for iteration and 
__aenter__/__aexit__.

> Another important point to consider: if the new Trio Stream ABCs are 
> *significantly different* from asyncio.Stream and would require us to alias 
> too many methods or to do heavy refactoring and deprecations, then Trio will 
> have to show some real world usage and traction of its APIs first.

It sounds like the only aliasing will be for the send data method; everything 
else either has different semantics, or has both the same semantics and the 
same name.* And there won't be any refactoring needed; the whole goal of this 
design is to make sure that any reasonable stream implementation can easily 
provide these methods :-).

*I was going to say the "send EOF" method would also be potentially aliased, 
but send EOF should be async, because e.g. on a TLS stream it has to send data, 
and Stream.write_eof is sync. Or maybe we should migrate Stream.write_eof to 
become async too?

> Nathaniel, I think it's important to emphasize that those compromises should 
> be mutual.  I'm willing to support changing "Stream.close()" to 
> "Stream.aclose()" and to perhaps alias some methods.  We can also implement 
> "Stream.chunks()".  But changing the semantics of "__aiter__" is, 
> unfortunately, not on the table, at least for me.

Let's put __aiter__ aside for a moment, and just think about what's best for 
asyncio itself. And if that turns out to include breaking compatibility between 
Stream and StreamReader/StreamWriter, then we can go back to the discussion 
about __aiter__ :-)

> Here's the plan:
>
> 1. We add "aclose()" and "write()" coroutines to the new "asyncio.Stream()".  
> It won't have "wait_closed()" or "drain()" or "close()".
>
> 2. We add a _LegacyStream class derived from the new asyncio.Stream.  We will 
> use it for subprocesses. Its "write()" method will return an 
> "OptionallyAwaitable" wrapper that will nudge users to add an await in front 
> of "stdin.write()".  _LegacyStream will be completely backwards compatible.
>
> This path enables us to add a decent new streaming API while retaining 
> consistency and backwards compatibility.

I don't think this is a terrible plan or anything like that. But I'm still 
confused about why you think it's better than adding new subprocess spawn 
functions. IIUC, your goal is to make the documentation and deprecations as 
simple as possible.

If we add two new subprocess functions, then the documentation/deprecations 
look like this:

- We have to document that there are two different versions of the stream API, 
the new one and the legacy one
- We have to document how Stream works, and how StreamReader/StreamWriter work
- We have to document that 6 functions that return old-style streams are 
deprecated (open_connection, start_server, open_unix_connection, 
start_unix_server, create_subprocess_exec, create_subprocess_shell) and 
replaced by 8 new functions (connect, StreamServer, connect_unix, 
UnixStreamServer, connect_read_pipe, connect_write_pipe, and whatever we called 
the new subprocess functions)

OTOH, with your proposal, we also have a set of deprecations and migrations to 
do:

- We have to document that there are two different versions of the stream API, 
the new one and the legacy one
- We have to document how Stream works, and how StreamReader/StreamWriter work
- We have to document that 4 functions that return old-style streams are 
deprecated (open_connection, start_server, open_unix_connection, 
start_unix_server) and replaced by 6 new functions (connect, StreamServer, 
connect_unix, UnixStreamServer, connect_read_pipe, connect_write_pipe)
- We have to document that Process objects use a third kind of stream object 
that doesn't match either the old or new APIs, and how this one works
- We 

[issue38242] Revert the new asyncio Streams API

2019-09-23 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

BTW Andrew, while writing that I realized that there's also overlap between 
your new Server classes and Trio's ABC for servers, and I think it'd be 
interesting to chat about how they compare maybe? But it's not relevant to this 
issue, so maybe gitter or another issue or something?

--

___
Python tracker 
<https://bugs.python.org/issue38242>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38242] Revert the new asyncio Streams API

2019-09-23 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I saw Yury on Saturday, and we spent some time working through the implications 
of different options here.

For context: the stream ABCs will be a bit different from most third-party 
code, because their value is proportional to how many projects adopt them. So 
some kind of central standardization is especially valuable. And, they'll have 
lots of implementors + lots of users, so they'll be hard to change regardless 
of whether we standardize them (e.g. even adding new features will be a 
breaking change). Part of the reason we've been aggressively iterating on them 
in Trio is because I know that eventually we need to end up with a minimal core 
that can never change again, so I want to make sure we find the right one :-).

So Yury and I are tentatively thinking we'll make some kind of PEP for the 3.9 
timescale, probably just for the core ABCs, maybe in the stdlib or maybe just 
as an informational PEP that we can use to convince people that this is "the 
python way" (like the WSGI PEP).

Now, the implications for the asyncio.Stream API in 3.8. This is tricky because 
we aren't sure of how everything will shake out, so we considered multiple 
scenarios.

First decision point: will asyncio.Stream implement the ABCs directly, or will 
you need some kind of adapter? If we go with an adapter, then there's no 
conflict between the ABC approach and whatever we do in 3.8, because the 
adapter can always fix things up later. But, it might be nicer to eventually 
have asyncio.Stream implement the ABC directly, so users can use the 
recommended API directly without extra fuss, so let's consider that too.

Next decision point: will the byte stream ABC have an __aiter__ that yields 
chunks? We're pretty sure this is the only place where the ABC *might* conflict 
with the asyncio.Stream interface. And as Josh Oreman pointed out in the Trio 
issue thread, we could even avoid this by making the ABC's chunk-iterator be a 
method like .chunks() instead of naming it __aiter__.

So even with the current asyncio.Stream, there are two potentially workable 
options. But, they do involve *some* compromises, so what would happen if we 
wanted asyncio.Stream to implement the ABC directly without and adapter, *and* 
the ABC uses __aiter__? We can't do that with the current asyncio.Stream. Are 
there any tweaks we'd want to make to 3.8 to keep our options open here?

The obvious change would be to leave out __aiter__ from asyncio.Stream in 3.8. 
That would leave all our options open. For sockets this would be easy, because 
the old functions are still there and still returning StreamReader/StreamWriter 
objects. For 3.8, we're adding a bunch of new Stream-based APIs, and users 
won't encounter a Stream until they switch to those. (The new APIs are: 
connect, connect_read_pipe, connect_write_pipe, connect_unix, StreamServer, 
UnixStreamServer). The problem is the subprocess functions 
(create_subprocess_exec, create_subprocess_shell), since they've been changed 
*in place* to return asyncio.Stream instead of StreamReader/StreamWriter.

We considered the possibility of migrating the existing subprocess functions to 
a different __aiter__ implementation via a deprecation period, but concluded 
that this wasn't really workable. So if we want to go down this branch of the 
decision tree, then 3.8 would have to leave create_subprocess_{exec,shell} as 
using StreamReader/StreamWriter, and in either 3.8 or 3.9 we'd have to add new 
subprocess functions that use Stream, like we did for sockets.

Doing this for subprocesses is a bit more complicated than for sockets, because 
subprocesses have a Process object that holds the stream objects and interacts 
with them. But looking at the code, I don't see any real blockers.

If we completely separated the old StreamReader/StreamWriter functions from the 
new Stream functions, then it would also have another advantage: we could clean 
up several issues with Stream that are only needed for compatibility with the 
old APIs. In particular, we could get rid of the optional-await hacks on 
'write' and 'close', and turn them into regular coroutines.

So I guess this is the real question we have to answer now. Which of these do 
we pick?

Option 1: Keep the Stream code as-is, and accept that using asyncio.Stream with 
future ABC-based code will require some compromises

Option 2: Create new functions for spawning subprocesses; revert 
create_subprocess_{exec,shell} to use StreamReader/StreamWriter; take advantage 
of the break between old and new APIs to clean up Stream in general; take 
advantage of that cleanup to remove __aiter__ so the the future ABC-based code 
won't have to compromise.

I think this is one of those good problems to have, where really we could live 
with either option :-). Still, we should pick one on purpose instead of by 
accident.

Yury's preference was "option 1", because he feels compromises for the ABC 
de

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

2019-09-22 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Just noticed this while looking at the code to asyncio.Task.__step

asyncio Futures have two different error states: they can have an arbitrary 
exception set, or they can be marked as cancelled.

asyncio Tasks handle this by detecting what exception was raised by the task 
code: if it's a CancelledError, then the mark the Future as cancelled, and if 
it's anything else, they set that as the Future's exception.

There is also a special case handled inside the 'except StopIteration' clause 
in Task.__step. If we request that a Task be cancelled, but then the task exits 
normally before we have a chance to throw in a CancelledError, then we also 
want mark the Future as cancelled. BUT, this special case is handled 
incorrectly: it does Future.set_exception(CancelledError()), instead of 
Future.cancel(). Normally it's impossible for a task to end up with its 
exception set to CancelledError, but it does happen in this one weird edge 
case, basically as a race condition.

Here's some sample code to illustrate the problem (tested on 3.7):

--

import asyncio

# This gets cancelled normally
async def cancel_early():
asyncio.current_task().cancel()
await asyncio.sleep(1)

async def cancel_late():
asyncio.current_task().cancel()
# No sleep, so CancelledError doesn't get delivered until after the
# task exits

async def main():
t_early = asyncio.create_task(cancel_early())
await asyncio.sleep(0.1)
print(f"t_early.cancelled(): {t_early.cancelled()!r}")

t_late = asyncio.create_task(cancel_late())
await asyncio.sleep(0.1)
print(f"t_late.cancelled(): {t_late.cancelled()!r}")
print(f"t_late.exception(): {t_late.exception()!r}")

asyncio.run(main())


--

Output:

t_early.cancelled(): True
t_late.cancelled(): False
t_late.exception(): CancelledError()

The obvious fix would be to modify the 'except StopIteration' branch to handle 
this case by calling super().cancel() instead of super().set_exception(...).

Alternatively, I could see an argument that asyncio.Task should always preserve 
the CancelledError, so that e.g. you can get a proper traceback. In that case 
we'd need to change the 'except CancelledError' branch to call 
super().set_exception(...) instead of super().cancel(). This would also need 
some more changes, like overriding .cancelled() to check for a stored exception 
and then return isinstance(stored_exc, CancelledError), and maybe others... I'm 
not sure of the full consequences.

But handling these two cases differently is definitely wrong, that part I'm 
sure of :-)

--
messages: 352964
nosy: asvetlov, njs, yselivanov
priority: normal
severity: normal
status: open
title: inconsistency in asyncio.Task between cancellation while running vs. 
cancellation immediately after it finishes

___
Python tracker 
<https://bugs.python.org/issue38248>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38242] Revert the new asyncio Streams API

2019-09-21 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> I think we need something *like* asyncio.Stream in the mix, for compatibility 
> and bridging between the two worlds, but right now it's not quite clear how 
> that should look, and pushing it off to 3.9 would give us time to figure out 
> the details for how these things can all fit together best.

Sorry, this was unclear. The "two worlds" I mean here are the current asyncio 
landscape and a future where we have standardized composable ABCs.

--
nosy:  -rhettinger

___
Python tracker 
<https://bugs.python.org/issue38242>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38242] Revert the new asyncio Streams API

2019-09-21 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

That seems a bit extreme, and I don't think conflicts with Trio are the most 
important motivation here. (And they shouldn't be.) But, we should probably 
write down what the motivations actually are.

Yury might have a different perspective, but to me the challenge of the asyncio 
stream API is that it's trying to do three things at once:

1. Provide a generic interface for representing byte streams
2. Provide useful higher-level tools for working with byte streams, like 
readuntil and TLS
3. Adapt the protocol/transports world into the async/await world

These are all important things that should exist. The problem is that 
asyncio.Stream tries to do all three at once in a single class, which makes 
them tightly coupled.

*If* we're going to have a single class that does all those things, then I 
think the new asyncio.Stream code does that about as well as it can be done.

The alternative is to split up those responsibilities: solve (1) by defining 
some simple ABCs to represent a generic stream, that are optimized to be easy 
to implement for lots of different stream types, solve (2) by building 
higher-level tools that work against the ABC interface so they work on any 
stream implementation, and then once that's done it should be pretty easy to 
solve (3) by implementing the new ABC for protocol/transports.

By splitting things up this way, I think we can do a better job at solving all 
the problems with fewer compromises. For example, there are lots of third-party 
libraries that use asyncio and want to expose some kind of stream object, like 
HTTP libraries, SSH libraries, SOCKS libraries, etc., but the current design 
makes this difficult. Also, building streams on top of protocols/transports 
adds unnecessary runtime overhead.

A further bonus is that (1) and (2) aren't tied to any async library, so it's 
possible to borrow the work that Trio's been doing on them, and to potentially 
share this part of the code between Trio and asyncio.

So that all sounds pretty cool, but what does it have to do with shipping 
asyncio.Stream in 3.8? The concern is that right now asyncio has two APIs for 
working with byte streams (protocols/transports + StreamReader/StreamWriter); 
then 3.8 adds asyncio.Stream; and then the design I'm describing will make a 
*fourth*, which is a lot. I think we need something *like* asyncio.Stream in 
the mix, for compatibility and bridging between the two worlds, but right now 
it's not quite clear how that should look, and pushing it off to 3.9 would give 
us time to figure out the details for how these things can all fit together 
best.

--
nosy:  -rhettinger

___
Python tracker 
<https://bugs.python.org/issue38242>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38242] Revert the new asyncio Streams API

2019-09-20 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

Yury asked me to weigh in here, since I guess between him and Andrew there's 
some uncertainty about whether reverting is the right choice or not. I can't 
answer that, but I can share some thoughts.

Unfortunately, I wasn't aware of the Stream PR when it was first being written 
and reviewed; I only discovered the change a month or two ago, by accident. If 
I had, I'd have said something like:

> This is definitely a major improvement over the old API, that fixes some 
> important issues, which is awesome. But, it only fixes some of the issues, 
> not all of them, and it's really difficult to change things in asyncio after 
> they ship. So there's a tricky question: do you want to ship this now so 
> users can start taking advantage of its improvements immediately, or do you 
> want to wait to make sure you don't have to do multiple rounds of changes?

Of course, now we're in a situation where it's already merged, which makes 
things more awkward. But maybe it will clarify things a bit to do a thought 
experiment: if the asyncio.Stream API was a PR that hadn't been merged yet and 
we were considering merging it – would you hit the green merge button, given 
what we know now, or would you hold off for 3.9?

--
nosy:  -xtreak

___
Python tracker 
<https://bugs.python.org/issue38242>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue30491] Add a lightweight mechanism for detecting un-awaited coroutine objects

2019-09-12 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It's something I'm still interested in, but I'm not actively working on it (as 
you noticed :-)), and there are some other CPython changes that I'll probably 
prioritize first. Do you want to close this and I can re-open it when I do get 
back to it, or...?

--

___
Python tracker 
<https://bugs.python.org/issue30491>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue38036] ssl docs say that ssl.SSLContext() is secure-by-default since 3.6, but it isn't

2019-09-05 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Quoting from the docs for ssl.SSLContext:

"Changed in version 3.6: The context is created with secure default values."
- https://docs.python.org/3/library/ssl.html#ssl.SSLContext

This is not true. If you call ssl.SSLContext(), you get a context with cert 
validation entirely disabled. And this has led to serious security bugs in 
practice: https://github.com/theelous3/asks/issues/134

Changing the defaults to make them actually secure would of course be nice, but 
is a complicated question that would need discussion. In the mean time, the 
docs shouldn't claim that it's secure. There should be a big bold note saying 
"UNLESS YOU HAVE A VERY GOOD REASON, DON'T USE THIS, USE 
ssl.create_default_connection()".

--
messages: 351186
nosy: alex, christian.heimes, dstufft, janssen, njs
priority: normal
severity: normal
status: open
title: ssl docs say that ssl.SSLContext() is secure-by-default since 3.6, but 
it isn't

___
Python tracker 
<https://bugs.python.org/issue38036>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37398] contextlib.ContextDecorator decorating async functions

2019-08-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

@Yury: depends on what you mean by "100% reliable" :-). Like I said above, I'm 
normally super against automagic detection of sync-vs-async functions because 
of all the edge cases where it goes wrong, but in this specific case where 
people are writing a decorator one line above their def/async def, I think a 
simple iscoroutinefunction check will be pretty close to 100% matching what 
users expect.

Or, if we don't change the semantics, then we can still be 100% confident that 
if iscoroutinefunction returns true, then the user has made a mistake. (I.e., 
if we make this issue a warning, then it's possible we'll miss print a warning 
in some complicated cases, but we can be confident that all the warnings we do 
print are correct.)

--

___
Python tracker 
<https://bugs.python.org/issue37398>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37398] contextlib.ContextDecorator decorating async functions

2019-08-09 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> I wouldn't be OK with magic switching in the behaviour of ContextDecorator 
> (that's not only semantically confusing, it's also going to make the 
> contextlib function wrappers even slower than they already are).

I hear you on the semantic confusion, but is a single check at definition time 
really that expensive? The runtime cost is zero.

> I'm also entirely unclear on what you would expect a synchronous context 
> manager to do when applied to an asynchronous function, as embedding an 
> "await" call inside a synchronous with statement is unlikely to end well.

It would be like:

async def blah():
with something():
await foo()

There's nothing weird about this; people write the long version all the time. 
You'd only do it when 'something()' doesn't do I/O, but there are lots of 
context managers that don't do I/O.

--

___
Python tracker 
<https://bugs.python.org/issue37398>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32912] Raise non-silent warning for invalid escape sequences

2019-08-04 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I think we haven't *actually* done a proper DeprecationWarning period for this. 
We tried, but because of the issue with byte-compiling, the warnings were 
unconditionally suppressed for most users -- even the users who are diligent 
enough to enable warnings and look at warnings in their test suites.

I can see a good argument for making the change, but if we're going to do it 
then it's obviously the kind of change that requires a proper deprecation 
period, and that hasn't happened.

Maybe .pyc files need to be extended to store a list of syntax-related 
DeprecationWarnings and SyntaxWarnings, that are re-issued every time the .pyc 
is loaded? Then we'd at least have the technical capability to deprecate this 
properly.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue32912>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37398] contextlib.ContextDecorator decorating async functions

2019-07-25 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

There are two different axes of sync/async here, and it's important not to mix 
them up: the context manager could be sync or async, and the function that's 
being decorated could be sync or async. This issue is about the case where you 
have a sync context manager, and it's decorating an async function.

The current behavior is definitely super confusing and not at all useful.

In general, I'm also very skeptical about using iscoroutinefunction to 
automatically change semantics, because of the issue Andrew mentions about 
failing to handle complicated objects like functools.partial instances or sync 
functions that return awaitables. In this *specific* case though, I'm not 
sure... maybe it actually is OK?

What's special about this case is that when you write:

@sync_cm()
async def foo():
...

...the same person is writing both the @ line and the 'async def' line, and 
they're right next to each other in the source code. So the normal concern 
about some unexpected foreign object being passed in doesn't really apply. And 
in fact, if I did have an awaitable-returning-function:

@sync_cm()
def foo():
return some_async_fn()

...then I think most people would actually *expect* this to be equivalent to:

def foo():
with sync_cm():
return some_async_fn()

Not very useful, but not surprising either. So maybe this is the rare case 
where switching on iscoroutinefunction is actually OK? The only cases where it 
would fail is if someone invokes the decorator form directly, e.g.

new_fn = sync_cm()(old_fn)

This seems pretty rare, but probably someone does do it, so I dunno.

If we think that's too magic, another option would be to make up some more 
explicit syntax, like:

@sync_cm().wrap_async
async def foo(): ...

...Oh, but I think this breaks the Guido's gut feeling rule 
(https://mail.python.org/pipermail/python-dev/2004-August/046711.html), so it 
would have to instead be:

@sync_cm.wrap_async()
async def foo(): ...

And even if we decide not to automatically switch on iscoroutinefunction, we 
should probably add a warning or something at least, because right now

@sync_cm()
async def ...

is definitely a mistake.

(Adding Nick to nosy as the contextlib maintainer.)

--
nosy: +ncoghlan, njs

___
Python tracker 
<https://bugs.python.org/issue37398>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37136] Travis CI: Documentation tests fails with Sphinx 2.1

2019-06-12 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

One of Guido's mentees (I don't know her bpo account name) also ran into this:

https://python.zulipchat.com/#narrow/stream/116742-core.2Fhelp/topic/Weird.20sphinx.20error

Some analysis shows that we do have a bug in the docs – the sphinx '.. 
module::' directive is only supposed to be used once per module, because it 
means "this is the canonical documentation for this module, that should be 
linked to whenever anyone mentions this module".

But we have two documents that use '.. module:: email.message':
  Doc/library/email.message.rst
  Doc/library/email.compat32-message.rst

I don't know enough about the email module to know what the right fix is, so 
I'll CC the email-interest-area folks too... maybe email.compat32-message.rst 
should be using '.. currentmodule:: email.message' instead of '.. module::'?

It's not clear why this error is appearing and disappearing for different 
people. The sphinx 2.1 release did make some changes in this area:

  
https://github.com/sphinx-doc/sphinx/commit/259be8716ad4b2332aa4d7693d73400eb06fa7d7

but that doesn't explain why it appears and disappears randomly.

Probably not worth tracking down though, given that we can just fix the 
underlying problem.

--
nosy: +barry, gvanrossum, maxking, njs, r.david.murray
resolution: not a bug -> 
stage: resolved -> 
status: closed -> open

___
Python tracker 
<https://bugs.python.org/issue37136>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28708] Low FD_SETSIZE limit on Windows

2019-06-12 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

Traditionally on Unix, sockets are represented by file descriptors. File 
descriptors are small integers. (POSIX actually mandates the "small" part: 
whenever a new fd is created, the OS has to assign it the smallest integer 
value that's not already being used.)

So when select was designed, they decided to be "clever" and use a bitmask to 
represent the FD sets. If all your fds have values less than 64, then you can 
use a single 64-bit integer to represent any arbitrary subset of them, sweet, 
it's super efficient. It's also extremely weird. No other API cares about the 
actual integer value of an fd, they're just opaque tokens. Select is almost 
unique in being O(highest fd value).

Of course this microoptimization stopped making sense decades ago, so poll() 
was added. The big innovation with poll() is that it takes an array of 
descriptors like a normal function, instead of this wacky bitmask thing. So its 
O(number of fds), and it doesn't matter whether you're checking fd #1 or fd 
#1000.

EXCEPT windows has a totally different history. On Windows, sockets are 
represented as handles. And handles are just like fds, EXCEPT that handles are 
allowed to have arbitrary values; they didn't copy POSIX's weird (and 
expensive) rule about alwaysv using the smallest possible integer.

So when Windows went to implement select(), the bitmask optimization never made 
any sense at all – even if you only have 1 socket, its handle might be, like, 
0x9f57be3a or something. So you'd need a bitmask with 2**32 entries, which is 
silly.

So on Windows, select() is secretly poll(). They copied the FD_* macros for 
compatibility, but fd_set is really just an array of opaque values + an 
explicit length, and you can pass in as many or as few as you want.

I know this is mostly rehashing conclusions that are in the thread already but 
I thought it might make more sense to have it laid out all together.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue28708>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37120] Provide knobs to disable session ticket generation on TLS 1.3

2019-06-05 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> Regarding your comment on client_context.num_ticket getter: IMHO it's not a 
> good idea to raise an exception on attribute access. It may break 
> introspection.

Hmm, I see what you mean.

Basically my intuition is: it's a bit weird to make the attribute's existence 
"sort of" depend on whether it's a client or server context. It would make 
sense to me to have it entirely disappear on client contexts (from __dir__, 
read-access, and write-access), and it would make sense to me to have it always 
be present, just a no-op. But having it present, and almost the same as on 
server contexts, except that assigning to it fails... that feels a little weird.

--
status: pending -> open

___
Python tracker 
<https://bugs.python.org/issue37120>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37120] Provide knobs to disable session ticket generation on TLS 1.3

2019-05-31 Thread Nathaniel Smith

New submission from Nathaniel Smith :

Maybe we should expose the SSL_CTX_set_num_tickets function:

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_num_tickets.html

This is a new function added in OpenSSL 1.1.1, that lets you control the number 
of tickets issued by a TLS 1.3 connection.

It turns out that by default, openssl 1.1.1 issues 2 tickets at the end of the 
server handshake. In principle this can cause deadlocks and data corruption:

  https://github.com/openssl/openssl/issues/7967
  https://github.com/openssl/openssl/issues/7948

And my problem specifically is that this pretty much kills all of Trio's fancy 
protocol testing helpers, because any protocol that's built on top of TLS is 
automatically broken as far as the helpers are concerned. And they're right. 
Right now I have to disable TLS 1.3 entirely to get Trio's test suite to avoid 
deadlocking.

Hopefully the openssl devs will fix this, but so far their latest comment is 
that they consider this a feature and so they think it has to stay broken for 
at least RHEL 8's lifetime.

Currently the only workaround is to either disable TLS 1.3, or disable tickets. 
But the 'ssl' module doesn't expose any way to control tickets. This issue 
proposes to add that.

Fortunately, exposing SSL_CTX_set_num_tickets should be pretty trivial, at 
least in theory.

Questions:

Do we have a preferred convention for how to expose these kinds of settings at 
the Python level? Attribute on SSLContext?

There's both an SSL* and a SSL_CTX* – I guess the CTX version is sufficient, or 
is there another convention?

As a bonus complication, openssl sometimes ignores the configured number of 
tickets, and uses a completely different mechanism:

> The default number of tickets is 2; the default number of tickets sent 
> following a resumption handshake is 1 but this cannot be changed using these 
> functions. The number of tickets following a resumption handshake can be 
> reduced to 0 using custom session ticket callbacks (see 
> https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_session_ticket_cb.html)

Do we want to open the can-of-worms involved in wrapping this too? I think if 
we only wrapped SSL_CTX_set_num_tickets then that would be enough to let me 
kluge my tests into passing and pretend that things are just fine, so long as 
we don't test session resumption...

--
assignee: christian.heimes
components: SSL
messages: 344148
nosy: alex, christian.heimes, dstufft, janssen, njs
priority: normal
severity: normal
status: open
title: Provide knobs to disable session ticket generation on TLS 1.3
type: enhancement
versions: Python 3.8, Python 3.9

___
Python tracker 
<https://bugs.python.org/issue37120>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue37006] Add top level await statement support for doctest

2019-05-22 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

As far as things like run/run_until_complete/etc go, I think doctests should 
have the same semantics as async REPLs, whatever those end up being.

Given that we don't actually have mature async REPLs, that the core feature to 
enable them only landed a few days ago, and the 3.8 freeze is almost here, I 
think we should defer the doctest discussion for 3.9. Hopefully by then, things 
will have settled down more in async REPL land, and it'll be more obvious what 
doctest should do.

--

___
Python tracker 
<https://bugs.python.org/issue37006>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34616] implement "Async exec"

2019-05-01 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> My response was to not take care of that in the first time, but provide the 
> building blocks for alternative REPL, in a second time provide an 
> async-input, and a way to register runner for alternative async libraries.

Yeah, I think this is pretty simple: the runtime/stdlib should provide the 
primitives to compile top-level async code, get coroutines, etc., and then a 
REPL like ipython can take care of handing that off to asyncio or whatever 
library they want.

Maybe in the long run the builtin REPL should get async support "out of the 
box", but that's much less clear, and anyway we should split that off into a 
separate issue if we want to discuss it.

async-input isn't even useful for ipython, is it? you use prompt-toolkit :-)

--

___
Python tracker 
<https://bugs.python.org/issue34616>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue36669] weakref proxy doesn't support the matrix multiplication operator

2019-04-24 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Yeah, seems like a simple oversight to me. (Actually this is the first I've 
heard of weakref.proxy...)

--

___
Python tracker 
<https://bugs.python.org/issue36669>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue36601] signals can be caught by any thread

2019-04-12 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

Yeah, the check makes sense on a system like the comment says IRIX used to be, 
where getpid() actually returns a thread id *and* signals are delivered to all 
threads (wtf). But any modern system should do the POSIX thing, where getpid() 
returns the same value in all threads, and signals are only delivered to one 
thread anyway. So I agree with everyone else that the original motivation 
doesn't make sense any more.

The place where this would change things is in fork/vfork context. For fork it 
seems unhelpful... like Greg notes, we already reset main_pid in the AfterFork 
handler, so the only effect is that there's a brief moment where signals can be 
lost. If we removed the (getpid() == main_thread) check, then fork() would work 
slightly better.

For vfork, man, I don't even know. Here I do disagree with Greg a little – 
according to POSIX, trip_signal is *not* safe to call in a vfork'ed process. 
POSIX says: "behavior is undefined if the process created by vfork() either 
modifies any data [...] or calls any other function". Yes this is really as 
extreme as it sounds – you're not allowed to mutate data or use any syscalls. 
And they explicitly note that this applies to signal handlers too: "If signal 
handlers are invoked in the child process after vfork(), they must follow the 
same rules as other code in the child process."

trip_signals sets a flag -> it mutates data -> it technically can invoke 
undefined behavior if it's called in a child process after a vfork. And it can 
call write(), which again, undefined behavior.

Of course this is so restrictive that vfork() is almost unusable in Python 
anyway, because you can't do anything in Python without modifying memory.

And worse: according to a strict reading of POSIX, vfork() should call 
pthread_atfork() handlers, and our pthread_atfork() handlers mutate memory.

So from the POSIX-rules-lawyer perspective, there's absolutely no way any 
Python process can ever call vfork() without invoking undefined behavior, no 
matter what we do here.

Do we care?

It looks like subprocess.py was recently modified to call posix_spawn in some 
cases: 
https://github.com/python/cpython/blob/a304b136adda3575898d8b5debedcd48d5072272/Lib/subprocess.py#L610-L654

If we believe the comments in that function, it only does this on macOS – where 
posix_spawn is a native syscall, so no vfork is involved – or on systems using 
glibc (i.e., Linux), where posix_spawn *does* invoke vfork(). So in this one 
case, currently, CPython does use vfork(). Also, users might call 
os.posix_spawn directly on any system.

However, checking the glibc source, their implementation of posix_spawn 
actually takes care of this – it doesn't use vfork() directly, but rather 
clone(), and it takes care to make sure that no signal handlers run in the 
child (see sysdeps/unix/sysv/linux/spawni.c for details).

AFAICT, the only ways that someone could potentially get themselves into 
trouble with vfork() on CPython are:

- by explicitly wrapping it themselves, in which case, good luck to them I 
guess. On Linux they aren't instantly doomed, because Linux intentionally 
deviates from POSIX and *doesn't* invoke pthread_atfork handlers after vfork(), 
but they still have their work cut out for them. Not really our problem.

- by explicitly calling os.posix_spawn on some system where this is implemented 
using vfork internally, but with a broken libc that doesn't handle signals or 
pthread_atfork correctly. Currently we don't know of any such systems, and if 
they do exist they have to be pretty rare.

So: I think we shouldn't worry about vfork(), and it's fine to remove the 
(getpid() == main_pid) check.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue36601>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33346] Syntax error with async generator inside dictionary comprehension

2019-03-06 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> Except that your first example (an asynchronous generator in a synchronous 
> comprehensions) is allowed in the current code and can occur inside a 
> synchronous function.

Oh yeah, you're right of course.

--

___
Python tracker 
<https://bugs.python.org/issue33346>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33346] Syntax error with async generator inside dictionary comprehension

2019-03-04 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

There are some tricky subtleties here around the distinction between 
list/dict/set comprehensions and generator expressions.

For list/dict/set comprehensions, they're evaluated eagerly, so an async 
comprehension can only occur in async context. For generator expressions, 
they're evaluated lazily, so you can write an async generator expression inside 
a non-async context. *Consuming* the async generator will require an async 
context, but all the expression itself does is instantiate an object, and 
that's synchronous.

(Like Guido, I'm not 100% sure that it's useful to support async generator 
expressions inside sync contexts, but we've already shipped it, so I'll assume 
we're going to keep it.)

So, I would expect the rule to be, precisely: if an async list/dict/set 
comprehension occurs inside either a list/dict/set comprehension or a generator 
expression, that should force the enclosing expression to become async.

So this is a synchronous comprehension, even though it has an async generator 
expression in it, and must occur inside an 'async def':

[(i async for i in range(j)) for j in range(n)]

And this is an async generator expression, which can legally be placed inside a 
regular 'def' (but can only be consumed inside an 'async def'):

([i async for i in range(j)] for j in range(n))

This might be what Yury/Serhiy/etc. already had in mind, but it's complicated 
enough that it seemed like it's worth spelling out in detail...

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue33346>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35886] Move PyInterpreterState into Include/internal/pycore_pystate.h

2019-02-25 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

The cffi issue is: 
https://bitbucket.org/cffi/cffi/issues/403/build-fails-on-38-dev-pyinterpreterstate

Armin already went ahead and committed the change to make cffi include 
internal/pycore_pystate.h. I guess this might not be the ideal final outcome 
though :-). Without some fix though it's very difficult to test things on 
3.8-dev, because cffi shows up in the dependency stacks of *large* portions of 
the ecosystem.

--

___
Python tracker 
<https://bugs.python.org/issue35886>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35886] Move PyInterpreterState into Include/internal/pycore_pystate.h

2019-02-24 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

This broke cffi:

gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 
-Wall -g -fstack-protector --param=ssp-buffer-size=4 -Wformat 
-Werror=format-security -fPIC -DUSE__THREAD -DHAVE_SYNC_SYNCHRONIZE 
-I/opt/python/3.8-dev/include/python3.8m -c c/_cffi_backend.c -o 
build/temp.linux-x86_64-3.8/c/_cffi_backend.o
In file included from c/cffi1_module.c:20:0,
 from c/_cffi_backend.c:7552:
c/call_python.c: In function ‘_get_interpstate_dict’:
c/call_python.c:20:30: error: dereferencing pointer to incomplete type 
‘PyInterpreterState {aka struct _is}’
 builtins = tstate->interp->builtins;
  ^
error: command 'gcc' failed with exit status 1

I haven't investigated further but heads up.

--
nosy: +arigo, njs

___
Python tracker 
<https://bugs.python.org/issue35886>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35894] Apparent regression in 3.8-dev: 'TypeError: required field "type_ignores" missing from Module'

2019-02-03 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Oh, that's not my code, it's the core of IPython's REPL :-). I just filed a bug 
with them, referencing this one: https://github.com/ipython/ipython/issues/11590

--

___
Python tracker 
<https://bugs.python.org/issue35894>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35894] Apparent regression in 3.8-dev: 'TypeError: required field "type_ignores" missing from Module'

2019-02-03 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Travis provides a "3.8-dev" python, which is updated regularly to track cpython 
master. When running our tests on this Python, specifically version

  python: 3.8.0a0 (heads/master:f75d59e, Feb  3 2019, 07:27:24) 

we just started getting tracebacks:

  TypeError Traceback (most recent call last)
  /opt/python/3.8-dev/lib/python3.8/codeop.py in __call__(self, source, 
filename, symbol)
  131 
  132 def __call__(self, source, filename, symbol):
  --> 133 codeob = compile(source, filename, symbol, self.flags, 1)
  134 for feature in _features:
  135 if codeob.co_flags & feature.compiler_flag:
  TypeError: required field "type_ignores" missing from Module

(Full log: https://travis-ci.org/python-trio/trio/jobs/488312057)

Grepping through git diffs for 'type_ignores' suggests that this is probably 
related to bpo-35766.

I haven't dug into this in detail, but it seems to be happening on tests using 
IPython. The lack of further traceback suggests to me that the exception is 
happening inside IPython's guts (it has some hacks to try to figure out which 
parts of the traceback are in user-defined code versus its own internal code, 
and tries to hide the latter when printing tracebacks). The crash is in 
codeop.Compile.__call__, and IPython does create ast.Module objects and pass 
them to codeop.Compile.__call__:

https://github.com/ipython/ipython/blob/512d47340c09d184e20811ca46aaa2f862bcbafe/IPython/core/interactiveshell.py#L3199-L3200

Maybe ast.Module needs to default-initialize the new type_ignores field, or 
compile() needs to be tolerant of it being missing?

--
messages: 334807
nosy: benjamin.peterson, brett.cannon, gvanrossum, njs, yselivanov
priority: normal
severity: normal
status: open
title: Apparent regression in 3.8-dev: 'TypeError: required field 
"type_ignores" missing from Module'
type: behavior
versions: Python 3.8

___
Python tracker 
<https://bugs.python.org/issue35894>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35684] Windows "embedded" Python downloads are malformed

2019-01-08 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

You're right, good catch!

--
resolution: duplicate -> fixed
stage: resolved -> 
superseder: Fatal Python error: initfsencoding: unable to load the file system 
codec zipimport.ZipImportError: can't find module 'encodings' -> 

___
Python tracker 
<https://bugs.python.org/issue35684>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35684] Windows "embedded" Python downloads are malformed

2019-01-08 Thread Nathaniel Smith


New submission from Nathaniel Smith :

~$ unzip -l /tmp/python-3.7.2-embed-amd64.zip 
Archive:  /tmp/python-3.7.2-embed-amd64.zip
  Length  DateTimeName
-  -- -   
99856  2018-12-23 23:10   python.exe
98320  2018-12-23 23:10   pythonw.exe
  3780624  2018-12-23 23:09   python37.dll
58896  2018-12-23 23:10   python3.dll
85232  2018-12-10 22:06   vcruntime140.dll
   200208  2018-12-23 23:10   pyexpat.pyd
26640  2018-12-23 23:10   select.pyd
  1073168  2018-12-23 23:10   unicodedata.pyd
28688  2018-12-23 23:10   winsound.pyd
71184  2018-12-23 23:10   _asyncio.pyd
89104  2018-12-23 23:10   _bz2.pyd
22544  2018-12-23 23:10   _contextvars.pyd
   133136  2018-12-23 23:10   _ctypes.pyd
   267792  2018-12-23 23:10   _decimal.pyd
   209424  2018-12-23 23:10   _elementtree.pyd
38928  2018-12-23 23:10   _hashlib.pyd
   257040  2018-12-23 23:10   _lzma.pyd
39440  2018-12-23 23:10   _msi.pyd
29200  2018-12-23 23:10   _multiprocessing.pyd
44048  2018-12-23 23:10   _overlapped.pyd
27664  2018-12-23 23:10   _queue.pyd
75792  2018-12-23 23:10   _socket.pyd
85520  2018-12-23 23:10   _sqlite3.pyd
   123408  2018-12-23 23:10   _ssl.pyd
  2480296  2018-12-23 22:20   libcrypto-1_1-x64.dll
   523944  2018-12-23 22:20   libssl-1_1-x64.dll
  1190416  2018-12-23 23:10   sqlite3.dll
85232  2018-12-10 22:06   vcruntime140.dll
  2386539  2018-12-23 23:14   python37.zip
   79  2018-12-23 23:14   python37._pth
- ---
 13632362 30 files

Notice that "vcruntime140.dll" appears twice on this list, once near the top 
and once near the bottom.

If we try to unpack this using the powershell Expand-Archive command, it fails 
with:

Failed to create file 'D:\a\1\s\python-dir\vcruntime140.dll' while expanding 
the archive file 
'D:\a\1\s\python.zip' contents as the file 
'D:\a\1\s\python-dir\vcruntime140.dll' already exists. Use the -Force 
parameter if you want to overwrite the existing directory 
'D:\a\1\s\python-dir\vcruntime140.dll' contents when 
expanding the archive file.

Probably it would be better to only include one copy of vcruntime140.dll :-)

--
assignee: steve.dower
messages: 333217
nosy: njs, steve.dower
priority: normal
severity: normal
status: open
title: Windows "embedded" Python downloads are malformed

___
Python tracker 
<https://bugs.python.org/issue35684>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28134] socket.socket(fileno=fd) does not work as documented

2018-12-28 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Am I right that this is considered fixed (or at least as fixed as it can be), 
and the issue should be closed?

Also, small note in case anyone stumbles across this in the future and is 
confused: Python does *not* handle this correctly on Windows. I suspect 
Christian was confused because there's an undocumented features on Windows 
where if you pass fileno=, then that 
correctly reinstantiates the socket object. But fileno= 
doesn't seem to do any special autodetection of type/family/proto.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue28134>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35568] Expose the C raise() function in the signal module, for use on Windows

2018-12-26 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

I'd like to see it in 3.8, but don't know if I'll get to it, and it'd be a good 
onramp issue for someone who wants to get into cpython development. So, let's 
put the keyword on it for now, and see what happens...

--
keywords: +easy (C)

___
Python tracker 
<https://bugs.python.org/issue35568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35568] Expose the C raise() function in the signal module, for use on Windows

2018-12-22 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Vladimir Matveev pointed out that there's already a wrapper in 
_testcapimodule.c: 
https://github.com/python/cpython/blob/f06fba5965b4265c42291d10454f387b76111f26/Modules/_testcapimodule.c#L3862-L3879

So if we do add this to signalmodule.c, we can drop that.

--

___
Python tracker 
<https://bugs.python.org/issue35568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35568] Expose the C raise() function in the signal module, for use on Windows

2018-12-22 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It probably doesn't matter too much either way, but almost all the 
signal-related wrappers are in signal. os.kill is the only exception AFAIK.

--

___
Python tracker 
<https://bugs.python.org/issue35568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue35568] Expose the C raise() function in the signal module, for use on Windows

2018-12-22 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Suppose we want to test how a program responds to control-C. We'll want to 
write a test that delivers a SIGINT to our process at a time of our choosing, 
and then checks what happens. For example, asyncio and Trio both have tests 
like this, and Trio even does a similar thing at run-time to avoid dropping 
signals in an edge case [1].

So, how can we deliver a signal to our process?

On POSIX platforms, you can use `os.kill(os.getpid(), signal.SIGINT)`, and that 
works fine. But on Windows, things are much more complicated: 
https://github.com/python/cpython/pull/11274#issuecomment-449543725

The simplest solution is to use the raise() function. On POSIX, raise(signum) 
is just a shorthand for kill(getpid(), signum). But, that's only on POSIX. On 
Windows, kill() doesn't even exist... but raise() does. In fact raise() is 
specified in C89, so *every* C runtime has to provide raise(), no matter what 
OS it runs on.

So, you might think, that's ok, if we need to generate synthetic signals on 
Windows then we'll just use ctypes/cffi to access raise(). *But*, Windows has 
multiple C runtime libraries (for example: regular and debug), and you have to 
load raise() from the same library that Python is linked against. And I don't 
know of any way for a Python script to figure out which runtime it's linked 
against. (I know how to detect whether the interpreter is configured in debug 
mode, but that's not necessarily the same as being linked against the debug 
CRT.) So on the one platform where you really need to use raise(), there's 
AFAICT no reliable way to get access to it.

This would all be much simpler if the signal module wrapped the raise() 
function, so that we could just do 'signal.raise_(signal.SIGINT)'. We should do 
that.

---

[1] Specifically, consider the following case (I'll use asyncio terminology for 
simplicity): (1) the user calls loop.add_signal_handler(...) to register a 
custom signal handler. (2) a signal arrives, and is written to the wakeup pipe. 
(3) but, before the loop reads the wakeup pipe, the user calls 
loop.remove_signal_handler(...) to remove the custom handler and restore the 
original signal settings. (4) now the loop reads the wakeup pipe, and discovers 
that a signal has arrived, that it no longer knows how to handle. Now what? In 
this case trio uses raise() to redeliver the signal, so that the new signal 
handler has a chance to run.

--
messages: 332382
nosy: njs, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: Expose the C raise() function in the signal module, for use on Windows
type: enhancement
versions: Python 3.8

___
Python tracker 
<https://bugs.python.org/issue35568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34271] Please support logging of SSL master secret by env variable SSLKEYLOGFILE

2018-12-20 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

It's confusing, but AFAICT what happened is that Mozilla started to disable it 
in release builds, but got a bunch of pushback from users and changed their 
minds and decided to keep it enabled. But then there was a snafu tracking the 
patch for that, so there ended up being a few releases where it was disabled, 
before everything got sorted out. But, it is enabled by default now. The very 
confusing thread is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1188657

And I don't see how this is any riskier than other envvars like PYTHONSTARTUP, 
which lets you name an arbitrary file that the interpreter will execute at 
startup.

--

___
Python tracker 
<https://bugs.python.org/issue34271>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34616] implement "Async exec"

2018-12-10 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> I'm thinking of submitting a talk at PyCon to explain what we've discover so 
> far in IPython.

You totally should!

Or actually there are two options to think about: you can submit a general 
talk, or submit a talk to the language summit. (Or write two talks and do both, 
I guess.) They're pretty different – the summit is a more informal thing (no 
video, smaller room), mostly just core devs, more of a working meeting kind of 
thing where you can argue about technical details.

--

___
Python tracker 
<https://bugs.python.org/issue34616>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34850] Emit a syntax warning for "is" with a literal

2018-11-30 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

> In my experience people are more likely to run code through a linter than 
> they are to ever run an interpreter with DeprecationWarning enabled.

This used to be true, and it was a disaster. So there's been a lot of work to 
fix it, and it's not true anymore.

Starting in 3.7, the built-in REPL has DeprecationWarning enabled by default, 
as do all standalone scripts (see PEP 565).

IPython/Jupyter has enabled DeprecationWarning by default for interactive code 
for about 3 years now: https://github.com/ipython/ipython/issues/8478

pytest started showing DeprecationWarnings by default a few months ago, and 
unittest has done so for ages.

I don't think I've ever met someone who skipped straight to linting, without 
ever having used a REPL, or written a script, or written a test :-)

--

___
Python tracker 
<https://bugs.python.org/issue34850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34850] Emit a syntax warning for "is" with a literal

2018-11-30 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Would it be more acceptable to use a DeprecationWarning? It's not really the 
right thing because we're not planning to ever actually remove the 
functionality. But, we've already gone to a lot of trouble to try to show 
DeprecationWarnings specifically to devs and not end users (hiding them by 
default, except if the source was the REPL or a test, getting patches into 
every test framework to handle this, etc.). And the issues here seem to be 
identical.

Or maybe LintWarning inherits from DeprecationWarning? Or the share a common 
superclass?

--

___
Python tracker 
<https://bugs.python.org/issue34850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue33457] python-config ldflags, PEP 513 and explicit linking to libpython in python extensions

2018-10-18 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

Also, python-config is inconsistent with distutils. It should link to libpython 
only in the cases where distutils does. (IIRC it's supposed to depend on 
whether python was built with --enable-shared.)

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue33457>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34670] Add set_post_handshake_auth for TLS 1.3

2018-10-17 Thread Nathaniel Smith


Nathaniel Smith  added the comment:

FYI Christian, your "typical scenario for HTTP" doesn't make sense to me... you 
can't send HTTP Connection Upgrade in the middle of a regular request/response 
cycle. I feel like the typical scenario ought to be more like:

* client
  * send ``HTTP GET /path``
* server
  * recv
  * verify_client_post_handshake (maybe... via calling SSL_do_handshake again?)
* client
  * recv
  * send upgrade confirmation (emits Certificate, CertificateVerify, Finish 
message)
* server
  * recv
  * verify certificate
  * send either the requested response, or a 401 Unauthorized depending

But I don't really understand the underlying design here, either at the TLS 1.3 
level or the openssl level, and haven't found very useful docs yet, so I could 
be wrong.

--
nosy: +njs

___
Python tracker 
<https://bugs.python.org/issue34670>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34968] loop.call_soon_threadsafe should be documented to be re-entrant-safe too

2018-10-14 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> What would make it not reentrant-safe?

Probably the most obvious example of a non-reentrant-safe operation is taking a 
lock. It's very natural to write code like:

def call_soon(...):
with self._call_soon_lock:
...

but now imagine that inside the body of that 'with' statement, a signal 
arrives, so the interpreter pauses what it's doing to invoke the signal 
handler, and the signal handler turns around and invokes call_soon, which tries 
to acquire the same lock that it already holds → instant deadlock.

And this rules out quite a few of the tools you might normally expect to use in 
thread-safe code, like queue.Queue, since they use locks internally.

The reason I think the stdlib's call_soon is OK is that it doesn't perform any 
blocking operations, and the critical operation is simply 
'self._ready.append(...)', which in CPython is atomic with respect to 
threads/signals/GC.

--

___
Python tracker 
<https://bugs.python.org/issue34968>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue34968] loop.call_soon_threadsafe should be documented to be re-entrant-safe too

2018-10-12 Thread Nathaniel Smith


New submission from Nathaniel Smith :

Asyncio needs a way to schedule work from other threads; and it also needs a 
way to scheduler work from code that can run at arbitrary times in the same 
thread, such as signal handlers or object finalizers ("reentrant contexts").

Currently loop.call_soon_threadsafe is documented to be the way to schedule 
work from other threads, but there is no documented way to schedule work from 
reentrant contexts. These are not quite the same thing, because reentrant 
contexts block the main thread while they're running. Generally, making code 
safe to call from __del__ or signal handlers is strictly harder than making it 
safe to call from other threads. (See bpo-14976 for an example of stdlib code 
being thread-safe but not reentrant-safe.)

Technically speaking, this means that right now, if you need to call an asyncio 
API from a __del__ method, then the only officially supported way to do that is 
to write something like:

def __del__(self):
def actual_cleanup_code():
...
def thread_dispatcher():
loop.call_soon_threadsafe(actual_cleanup_code)
thread = threading.Thread(target=thread_dispatcher)
thread.start()

But this is kind of silly. There should be some equivalent of loop.call_soon 
that *is* safe to call from reentrant contexts, so we could just write:

def __del__(self):
def actual_cleanup_code():
...
loop.call_soon_reentrant_safe(actual_cleanup_code)

But... it doesn't really make sense to add a new method for this, since the 
desired semantics are strictly more powerful than the current 
loop.call_soon_threadsafe. Instead, we should tighten the guarantees on 
call_soon_threadsafe, by documenting that it's safe to use from reentrant 
contexts.

Also, AFAICT the stdlib's implementation of call_soon_threadsafe is already 
reentrant-safe, so this wouldn't require any changes to stdlib code, only to 
the docs. But it would provide an additional formal guarantee that user-level 
code could take advantage of, and impose an additional constraint on developers 
of third-party loops.

(I don't think the constraint is *too* onerous, fortunately. It's quite tricky 
to implement a version of call_soon that's thread-safe, reentrant-safe, *and* 
guarantees that the callback will eventually be invoked, even if call_soon 
races with loop shutdown. But in asyncio, all variants of call_soon are allowed 
to silently drop callbacks at loop shutdown, which makes this much easier.)

--
components: asyncio
messages: 327618
nosy: asvetlov, njs, yselivanov
priority: normal
severity: normal
status: open
title: loop.call_soon_threadsafe should be documented to be re-entrant-safe too
versions: Python 3.8

___
Python tracker 
<https://bugs.python.org/issue34968>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



  1   2   3   4   5   >