[issue37658] In some cases asyncio.wait_for can lead to socket leak.

2022-03-11 Thread Sam Bull


Sam Bull  added the comment:

There is still an issue here. I proposed a possible solution at: 
https://github.com/python/cpython/pull/28149#issuecomment-1007823782

You can also scroll up through the lengthy discussion to see how I reached that 
conclusion. I've not had time yet to actually try implementing something yet.

--

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



[issue46771] Add some form of cancel scopes

2022-02-20 Thread Sam Bull


Sam Bull  added the comment:

> We could store the nonces in a single CancelledError instead.

Indeed, my initial proposal was exactly that, but after learning about 
ExceptionGroup, I thought that was a cleaner approach.

--

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



[issue46771] Add some form of cancel scopes

2022-02-20 Thread Sam Bull


Sam Bull  added the comment:

> Previously, when the task was cancelled twice, only one CancelledError was 
> raised. Now it would raise a BaseExceptionGroup instead.

I was under the impression that ExceptionGroup was somewhat backwards 
compatible, in that you could still use `except CancelledError:` and it would 
catch all the errors in the group. But, maybe I'm wrong, I've not seen the 
documentation for the final feature yet, but that's the impression I got from 
the PEP.

--

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



[issue46771] Add some form of cancel scopes

2022-02-20 Thread Sam Bull


Sam Bull  added the comment:

Actually, in your counter proposal, you say:

> Such context managers should still keep track of whether they cancelled the 
> task themselves or not, and if they did, they should always call t.uncancel().

But, if we are using nonces on the CancelledError to keep track, then only 1 
context manager will know if it was themselves or not. This is exactly why I'm 
proposing to use multiple CancelledErrors, so that every nonce is passed to the 
handling code.

--

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



[issue46771] Add some form of cancel scopes

2022-02-20 Thread Sam Bull


Sam Bull  added the comment:

> This should be solved when using the cancel count -- the explicit cancel 
> bumps the cancel count so the cancel scope (i.e. timeout()) will not raise 
> TimeoutError.

It will probably work in this case. But, what about more complex cases? If 
there are 2 different timeouts and the possibility of a user cancellation, and 
we have a count of 2 cancellations, then what happened? 2 timeouts, or 1 
timeout and user cancellation? Without being able to check the nonces of each 
cancellation, I don't see how this would work. Or if the user calls .cancel() 
twice explicitly, then you cancel both timeouts, even though it had nothing to 
do with the timeout.

Propagating an ExceptionGroup where every exception can be inspected to see if 
it was caused by this code or not still seems like the safe option to me (and 
obviously still has the cancel count implicitly).

--

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



[issue46771] Add some form of cancel scopes

2022-02-20 Thread Sam Bull


Sam Bull  added the comment:

> If the task is already pending a cancellation targeted at a cancel scope, the 
> task itself cannot be cancelled anymore since calling cancel() again on the 
> task is a no-op. This would be solved by updating the cancel message on the 
> second call.

> I think Andrew missed one case: in his second diagram, what if the explicit 
> cancel() happened *after* the timeout (but still in the same iteration)? 
> That's the case that makes me want to delete those two lines from 
> Task.cancel() (see my earlier message).

To expand on this point, I've been looking at solving the race conditions in 
async-timeout. To see how such a race condition can end up with a task never 
exiting, take a look at this example: 
https://github.com/aio-libs/async-timeout/issues/229#issuecomment-908502523

In the condition Guido describes, the user's cancellation is suppressed and the 
code runs forever.

I also wrote tests that seem to reliably reproduce the race condition (the 2nd 
one still seems unfixable with the current solutions, the 1st was fixed with 
the nonce/sentinel trick): 
https://github.com/aio-libs/async-timeout/commit/ab04eb53dcf49388b6e6eacf0a50bafe19c5c74b#diff-60a009a48129ae41018d588c32a6d94c54d1d2948cbc3b831fc27a9c8fdbac68L364-L421

You can see the flow of execution from the call_order assert at the end.

I think most of the solutions proposed here will still not solve this race 
condition. I initially proposed a solution at: 
https://bugs.python.org/issue45098

In short, I think that every time we call .cancel(), we need to raise another 
CancelledError. So, in this race condition you would get 2 CancelledErrors (via 
an ExceptionGroup). Then our code can catch the error with our nonce/sentinel 
and handle that, but also reraise any other errors which are unrelated.

--
nosy: +dreamsorcerer

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



[issue45098] asyncio.CancelledError should contain more information on cancellations

2022-02-19 Thread Sam Bull


Sam Bull  added the comment:

I think there's still a problem, in that the user still expects a task to be 
cancelled in the example previously: 
https://github.com/aio-libs/async-timeout/issues/229#issuecomment-908502523

If we encounter the race condition where the timeout cancels the task and then 
the user cancels the task, then we still have the case that async-timeout 
swallows the cancellation and the task will run forever. This would basically 
require the user to check everytime they want to cancel the task, with 
something awkward like:

```
while not task.cancel() and not task.cancelled():
await asyncio.sleep(0)
```

I think this change is still necessary, but rather than adding multiple values 
to e.args, we can use the new ExceptionGroup to raise multiple CancelledErrors. 
So, each call of task.cancel() will create a new CancelledError, and then all 
of those CancelledErrors will get raised together.

For async-timeout, we can then just catch the CancelledError with our sentinel 
and raise a TimeoutError, while reraising any other CancelledErrors.

--

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



[issue42130] AsyncIO's wait_for can hide cancellation in a rare race condition

2021-12-02 Thread Sam Bull


Sam Bull  added the comment:

It has been addressed, PR should be merged this week: 
https://github.com/python/cpython/pull/28149

Like most open source projects, it just needed someone to actually propose a 
fix.

--

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



[issue37658] In some cases asyncio.wait_for can lead to socket leak.

2021-10-25 Thread Sam Bull

Sam Bull  added the comment:

Can I get a review? 
https://github.com/python/cpython/pull/29202

Seems like a simple mistake given the original description of this issue:

> 1. the inner task is completed and the outer task will receive the result – 
> transport and protocol in this case
> 2. The inner task is cancelled and no connection was established

The try/except blocks clearly add a 3rd condition, where the inner task is 
completed and a TimeoutError is raised without returning the result.

--

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



[issue37658] In some cases asyncio.wait_for can lead to socket leak.

2021-10-24 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer
nosy_count: 10.0 -> 11.0
pull_requests: +27471
pull_request: https://github.com/python/cpython/pull/29202

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



[issue45098] asyncio.CancelledError should contain more information on cancellations

2021-09-04 Thread Sam Bull


New submission from Sam Bull :

There are awkward edge cases caused by race conditions when cancelling tasks 
which make it impossible to reliably cancel a task.

For example, in the async-timeout library there appears to be no way to avoid 
suppressing an explicit t.cancel() if that cancellation occurs immediately 
after the timeout.

In the alternative case where a cancellation happens immediately before the 
timeout, the solutions feel dependant on the internal details of how 
asynico.Task works and could easily break if the behaviour is tweaked in some 
way.

What we really need to know is how many times a task was cancelled as a cause 
of the CancelledError and ideally were the cancellations caused by us.

The solution I'd like to propose is that the args on the exception contain all 
the messages of every cancel() call leading up to that exception, rather than 
just the first one.

e.g. In these race conditions e.args would look like (None, SENTINEL), where 
SENTINEL was sent in our own cancellations. From this we can see that the task 
was cancelled twice and only one was caused by us, therefore we don't want to 
suppress the CancelledError.

For more details to fully understand the problem:
https://github.com/aio-libs/async-timeout/pull/230
https://github.com/aio-libs/async-timeout/issues/229#issuecomment-908502523
https://github.com/aio-libs/async-timeout/pull/237

--
components: asyncio
messages: 401045
nosy: asvetlov, dreamsorcerer, yselivanov
priority: normal
severity: normal
status: open
title: asyncio.CancelledError should contain more information on cancellations
type: behavior
versions: Python 3.10, Python 3.11, Python 3.9

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



[issue42130] AsyncIO's wait_for can hide cancellation in a rare race condition

2021-09-03 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer
nosy_count: 8.0 -> 9.0
pull_requests: +26587
pull_request: https://github.com/python/cpython/pull/28149

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



[issue44815] asyncio.gather no DeprecationWarning if task are passed

2021-08-02 Thread Sam Bull


Change by Sam Bull :


--
pull_requests: +26076
pull_request: https://github.com/python/cpython/pull/27569

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



[issue44815] asyncio.gather no DeprecationWarning if task are passed

2021-08-02 Thread Sam Bull


Sam Bull  added the comment:

There is another issue with asyncio.sleep() too, if the passed argument is 0. 
This also tripped up the tests in aiohttp (though I've also used an explicit 0 
in production code to yield back to the loop).

--

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



[issue44815] asyncio.gather no DeprecationWarning if task are passed

2021-08-02 Thread Sam Bull


Change by Sam Bull :


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

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



[issue44815] asyncio.gather no DeprecationWarning if task are passed

2021-08-02 Thread Sam Bull


New submission from Sam Bull :

When calling asyncio.gather() a DeprecationWarning is only emitted if no tasks 
are passed (which is probably the exceptional case, rather than the standard 
one).

This has resulted in us missing this deprecated argument in aiohttp until we 
received a bug report from a user trying it out against the 3.10 beta.

For some reason the warning only appears under a `if not coros_or_futures:` 
block. I think it should be run regardless:
https://github.com/python/cpython/blob/3.9/Lib/asyncio/tasks.py#L757

--
components: asyncio
messages: 398794
nosy: asvetlov, dreamsorcerer, yselivanov
priority: normal
severity: normal
status: open
title: asyncio.gather no DeprecationWarning if task are passed
versions: Python 3.8, Python 3.9

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



[issue43404] No SSL certificates when using the Mac installer

2021-03-04 Thread Sam Bull


New submission from Sam Bull :

After installing the latest version of Python on Mac OS X using the installer 
downloaded from python.org 
(https://www.python.org/ftp/python/3.9.2/python-3.9.2-macosx10.9.pkg), the 
installed version of Python is unable to find the system certificates.

Using the old version of Python located at 
/usr/local/Cellar/python/3.7.5/bin/python3, I get:

>>> ssl.create_default_context().cert_store_stats()
{'x509': 168, 'crl': 0, 'x509_ca': 168}

But, with the new version located at 
/Library/Frameworks/Python.framework/Versions/3.9/bin/python3, I get:

>>> ssl.create_default_context().cert_store_stats()
{'x509': 0, 'crl': 0, 'x509_ca': 0}


Looking around on the internet, this seems to be a pretty common issue on Mac, 
but is often getting misdiagnosed as an actual problem with the server's 
certificate. Because of that, nobody seems to have proposed any methods to fix 
it.

Examples:
https://github.com/aio-libs/aiohttp/issues/5375
https://stackoverflow.com/questions/65039677/unable-to-get-local-issuer-certificate-mac-os#comment115039330_65040851

--
assignee: christian.heimes
components: SSL
messages: 388123
nosy: christian.heimes, dreamsorcerer
priority: normal
severity: normal
status: open
title: No SSL certificates when using the Mac installer
type: behavior
versions: Python 3.9

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



[issue33315] Allow queue.Queue to be used in type annotations

2020-06-12 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer

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



[issue38364] inspect.iscoroutinefunction / isgeneratorfunction / isasyncgenfunction can't handle partialmethod objects

2020-06-07 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer

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



[issue40563] Support pathlike objects on dbm/shelve

2020-06-02 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer

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



[issue33618] Support TLS 1.3

2020-05-31 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer

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



[issue36484] Can't reorder TLS 1.3 ciphersuites

2020-05-31 Thread Sam Bull


Change by Sam Bull :


--
nosy: +dreamsorcerer

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



[issue8797] urllib2 basicauth broken in 2.6.5: RuntimeError: maximum recursion depth exceeded in cmp

2011-01-20 Thread Sam Bull

Sam Bull s...@pocketuniverse.ca added the comment:

I think there's a much simpler solution to this ticket than the retry logic 
that's currently in place.

The code originally avoided the infinite recursion by checking to see if the 
previous request had already submitted the auth credentials that would be used 
in the retry. If it had, it would return None. If it hadn't, it would add the 
auth credentials to the request header and the request again:

if req.headers.get(self.auth_header, None) == auth:
return None
req.add_header(self.auth_header, auth)

Then, to fix #3819, it was changed. Instead of calling add_header, it called 
add_unredirected_header:

if req.headers.get(self.auth_header, None) == auth:
return None
req.add_unredirected_header(self.auth_header, auth)

This caused the loop because the auth creds were going into unredirected_hdrs 
instead of the headers dict.

But I think the original logic is sound. The code just wasn't checking in all 
the headers. Luckily there's a get_header method that checks both for you. This 
one-line change should fix the issue:


if req.get_header(self.auth_header, None) == auth:
return None
req.add_unredirected_header(self.auth_header, auth)

I think this fix is cleaner and makes more sense, but I'm worried I might be 
missing something. I don't fully understand the distinction between headers and 
unredirected headers. Maybe there's a reason why the code isn't checking in 
unredirected headers for the auth header.

I'm attaching a patch. I'm new to contributing to python so I apologize if the 
format is wrong.

--
nosy: +sambull
versions: +Python 2.7
Added file: http://bugs.python.org/file20471/simpler_fix.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8797
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com