Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(170)

#25593: _sock_connect_cb can be called twice resulting in InvalidStateError

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by thehesiod
Modified:
2 years ago
Reviewers:
guido, yselivanov
CC:
gvanrossum, asvetlov, devnull_psf.upfronthosting.co.za, Yury Selivanov, thehesiod, tooker_gmail.com
Visibility:
Public.

Patch Set 1 #

Total comments: 7

Patch Set 2 #

Total comments: 2

Patch Set 3 #

Total comments: 1

Patch Set 4 #

Total comments: 1

Patch Set 5 #

Total comments: 1

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/asyncio/base_events.py View 1 2 3 4 5 7 chunks +9 lines, -16 lines 0 comments Download
Lib/asyncio/test_utils.py View 1 2 3 4 5 1 chunk +6 lines, -5 lines 0 comments Download
Lib/test/test_asyncio/test_base_events.py View 1 2 3 4 5 1 chunk +53 lines, -0 lines 0 comments Download

Messages

Total messages: 11
gvanrossum
Basically looks good. (But we do have a github repo for just asyncio which is ...
2 years ago #1
Yury Selivanov
A question: should 'loop._stopped(ing)' affect 'loop.is_running()'? http://bugs.python.org/review/25593/diff/15968/Lib/asyncio/base_events.py File Lib/asyncio/base_events.py (right): http://bugs.python.org/review/25593/diff/15968/Lib/asyncio/base_events.py#newcode180 Lib/asyncio/base_events.py:180: self._stopped = None ...
2 years ago #2
gvanrossum
Justin, can you revise your diff based on the feedback? http://bugs.python.org/review/25593/diff/15968/Lib/asyncio/test_utils.py File Lib/asyncio/test_utils.py (left): http://bugs.python.org/review/25593/diff/15968/Lib/asyncio/test_utils.py#oldcode73 ...
2 years ago #3
Yury Selivanov
http://bugs.python.org/review/25593/diff/15983/Lib/test/test_asyncio/test_base_events.py File Lib/test/test_asyncio/test_base_events.py (right): http://bugs.python.org/review/25593/diff/15983/Lib/test/test_asyncio/test_base_events.py#newcode804 Lib/test/test_asyncio/test_base_events.py:804: test_utils.run_once(self.loop) Maybe instead of testing "test_utils.run_once" function, which isn't ...
2 years ago #4
gvanrossum
http://bugs.python.org/review/25593/diff/15983/Lib/test/test_asyncio/test_base_events.py File Lib/test/test_asyncio/test_base_events.py (right): http://bugs.python.org/review/25593/diff/15983/Lib/test/test_asyncio/test_base_events.py#newcode804 Lib/test/test_asyncio/test_base_events.py:804: test_utils.run_once(self.loop) On 2015/11/19 18:35:16, Yury.Selivanov wrote: > Maybe instead ...
2 years ago #5
Yury Selivanov
> I think I'll pass on the warning after all. Why? Issuing a warning is ...
2 years ago #6
gvanrossum
On 2015/11/19 20:39:27, Yury.Selivanov wrote: > > I think I'll pass on the warning after ...
2 years ago #7
Yury Selivanov
looks good (besides one suggestion) http://bugs.python.org/review/25593/diff/15986/Lib/test/test_asyncio/test_base_events.py File Lib/test/test_asyncio/test_base_events.py (right): http://bugs.python.org/review/25593/diff/15986/Lib/test/test_asyncio/test_base_events.py#newcode816 Lib/test/test_asyncio/test_base_events.py:816: self.loop.run_forever() Maybe we should ...
2 years ago #8
gvanrossum
On 2015/11/19 21:18:09, Yury.Selivanov wrote: > looks good (besides one suggestion) > > http://bugs.python.org/review/25593/diff/15986/Lib/test/test_asyncio/test_base_events.py > ...
2 years ago #9
Yury Selivanov
http://bugs.python.org/review/25593/diff/15988/Lib/test/test_asyncio/test_base_events.py File Lib/test/test_asyncio/test_base_events.py (right): http://bugs.python.org/review/25593/diff/15988/Lib/test/test_asyncio/test_base_events.py#newcode811 Lib/test/test_asyncio/test_base_events.py:811: self.assertEqual(self.loop._selector.call_count, 0) It should be "self.loop._selector.select.assert_called_with(0)"
2 years ago #10
Yury Selivanov
2 years ago #11
LGTM
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7