[issue45098] asyncio.CancelledError should contain more information on cancellations
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
[issue46771] Add some form of cancel scopes
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
[issue46771] Add some form of cancel scopes
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
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
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
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
[issue8797] urllib2 basicauth broken in 2.6.5: RuntimeError: maximum recursion depth exceeded in cmp
Sam Bull 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 <http://bugs.python.org/issue8797> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37658] In some cases asyncio.wait_for can lead to socket leak.
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
[issue44815] asyncio.gather no DeprecationWarning if task are passed
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
[issue44815] asyncio.gather no DeprecationWarning if task are passed
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
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
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
[issue42130] AsyncIO's wait_for can hide cancellation in a rare race condition
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
[issue45098] asyncio.CancelledError should contain more information on cancellations
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
[issue37658] In some cases asyncio.wait_for can lead to socket leak.
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
[issue37658] In some cases asyncio.wait_for can lead to socket leak.
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
[issue42130] AsyncIO's wait_for can hide cancellation in a rare race condition
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
[issue43404] No SSL certificates when using the Mac installer
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
[issue36484] Can't reorder TLS 1.3 ciphersuites
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
[issue33618] Support TLS 1.3
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
[issue40563] Support pathlike objects on dbm/shelve
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
[issue38364] inspect.iscoroutinefunction / isgeneratorfunction / isasyncgenfunction can't handle partialmethod objects
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
[issue33315] Allow queue.Queue to be used in type annotations
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
