Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.5.0b3 Windows accept() on unready non-blocking socket raises PermissionError [now need unit test] #68920

Closed
bladebryan mannequin opened this issue Jul 27, 2015 · 10 comments
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@bladebryan
Copy link
Mannequin

bladebryan mannequin commented Jul 27, 2015

BPO 24732
Nosy @pfmoore, @pitrou, @vstinner, @larryhastings, @tjguk, @zware, @serhiy-storchaka, @eryksun, @zooba, @bladebryan

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-27.23:56:21.068>
created_at = <Date 2015-07-27.02:13:47.970>
labels = ['type-bug', 'OS-windows']
title = '3.5.0b3 Windows accept() on unready non-blocking socket raises PermissionError [now need unit test]'
updated_at = <Date 2019-05-27.23:56:21.067>
user = 'https://github.com/bladebryan'

bugs.python.org fields:

activity = <Date 2019-05-27.23:56:21.067>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-27.23:56:21.068>
closer = 'vstinner'
components = ['Windows']
creation = <Date 2015-07-27.02:13:47.970>
creator = 'bryangeneolson'
dependencies = []
files = []
hgrepos = []
issue_num = 24732
keywords = []
message_count = 10.0
messages = ['247452', '247453', '247475', '247479', '247489', '247491', '247494', '247495', '248303', '341888']
nosy_count = 12.0
nosy_names = ['paul.moore', 'pitrou', 'vstinner', 'larry', 'tim.golden', 'neologix', 'python-dev', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'bryangeneolson']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24732'
versions = ['Python 3.5', 'Python 3.6']

@bladebryan
Copy link
Mannequin Author

bladebryan mannequin commented Jul 27, 2015

In Python 3.4 on Windows 7, the code:

    import socket
    sock = socket.socket()
    sock.bind(('127.0.0.1', 52384))
    sock.listen(5)
    sock.setblocking(False)
    csock, addr = sock.accept()

Raised:

    Traceback (most recent call last):
      File "socket_bug_test.py", line 8, in <module>
        csock, addr = sock.accept()
      File "c:\bin\Python34\lib\socket.py", line 187, in accept
        fd, addr = self._accept()
    BlockingIOError: [WinError 10035] A non-blocking socket operation could not be completed immediately

In Python 3.5.0b3 on the same system, it is raising:

    Traceback (most recent call last):
      File "socket_bug_test.py", line 8, in <module>
        csock, addr = sock.accept()
      File "c:\bin\Python35\lib\socket.py", line 195, in accept
        fd, addr = self._accept()
    PermissionError: [WinError 5] Access is denied

This is a problem for other parts of the Standard Library. I hit it playing with asyncio, where in Lib\asyncio\selector_events.py the function BaseSelectorEventLoop._sock_accept() is prepared for an unready socket to raise BlockingIOError or InterruptedError, but does not catch the WinError:

    try:
        conn, address = sock.accept()
        conn.setblocking(False)
    except (BlockingIOError, InterruptedError):
        self.add_reader(fd, self._sock_accept, fut, True, sock)
    except Exception as exc:
        fut.set_exception(exc)
    else:
        fut.set_result((conn, address))

There are other calls to accept in accept() in asyncio, that I haven't tested but also look adversely affected.

The older asyncore module looks to have a similar problem. In dispatcher.accept():

    def accept(self):
        # XXX can return either an address pair or None
        try:
            conn, addr = self.socket.accept()
        except TypeError:
            return None
        except OSError as why:
            if why.args[0] in (EWOULDBLOCK, ECONNABORTED, EAGAIN):
                return None
            else:
                raise
        else:
            return conn, addr

The 'except OSError as why' will catch the WinError, but why.args[0] will be errno.EACCES = 13, permission denied, which is not equal to any of anticipated errors.

My system according to Python:

    >>> import sys
    >>> sys.version
    '3.5.0b3 (default, Jul  5 2015, 07:00:46) [MSC v.1900 64 bit (AMD64)]'
    >>> sys.getwindowsversion()
    sys.getwindowsversion(major=6, minor=1, build=7601, platform=2, service_pack='Service Pack 1')

That's Windows 7 Professional, 64-bit, with Service pack 1. It has an AND Phenom II X6 1055T Processor 2.8 GHz, and 8GB of memory.

@bladebryan bladebryan mannequin added OS-windows type-bug An unexpected behavior, bug, or error labels Jul 27, 2015
@zware
Copy link
Member

zware commented Jul 27, 2015

Thanks for the report! I'm pretty sure I've noticed the effects of this, but hadn't had a chance to look into it. With your nice small test case, I can start bisecting and try to figure out what caused it.

@zware
Copy link
Member

zware commented Jul 27, 2015

According to hg bisect:

The first bad revision is:
changeset: 95361:358a2bcd0d0b
user: Victor Stinner <victor.stinner@gmail.com>
date: Wed Apr 01 21:57:09 2015 +0200
summary: Issue bpo-23834: Add sock_call() helper function

Nosy list from that issue added here.

I don't understand that patch well enough to suggest any kind of fix, but I'm happy to run tests for anyone who can fix it.

I think this is a serious enough issue to block the next pre-release of 3.5.

@eryksun
Copy link
Contributor

eryksun commented Jul 27, 2015

The permission error comes from calling SetHandleInformation on an invalid socket value. The code shouldn't make it that far. The problem starts earlier in sock_accept_impl. This function checks whether SOCKET_T ctx->result is non-negative to determine whether the accept call succeeded, but SOCKET_T is unsigned on Windows.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Jul 27, 2015

New changeset cd60eccaa331 by Victor Stinner in branch '3.5':
Issue bpo-24732, bpo-23834: Fix sock_accept_impl() on Windows
https://hg.python.org/cpython/rev/cd60eccaa331

@vstinner
Copy link
Member

According to hg bisect: (...)
summary: Issue bpo-23834: Add sock_call() helper function

Oh, I'm not too surprised. I had to modify _deeply_ socketmodule.c to implement the PEP-475. I stressed the code on Linux, but on Windows I only ran Python test suite and asyncio test suite.

The surprising part is that the bug was not detected by any test :-(

We need at least one more unit test for it.

The problem starts earlier in sock_accept_impl. This function checks whether SOCKET_T ctx->result is non-negative to determine whether the accept call succeeded, but SOCKET_T is unsigned on Windows.

Thanks, good analysis. I checked accept() documentation and Python 3.4 source code: yeah, we must check for INVALID_SOCKET. It's a dummy copy/paste failure. I copied the code from another sock_xxx_impl() function, and I didn't notice that the error condition is different for accept(). The code fails in some cases on Windows, but not always. Maybe only on 64-bit?

My system according to Python:
...
'3.5.0b3 (default, Jul 5 2015, 07:00:46) [MSC v.1900 64 bit (AMD64)]'

My Windows is also 64-bit but my Visual Studio version (2010 Express) was limited to 32-bit :-(

Right now, I cannot develop on Windows: Visual Studio 2010 doesn't want to compile Python anymore (issue bpo-24737). I'm downloading Windows 8.1 and I will try Visual Studio 2015. So I pushed a fix without being able to test it :-(

@eryksun
Copy link
Contributor

eryksun commented Jul 27, 2015

LGTM on Windows 7:

    Python 3.5.0b4+ (default, Jul 27 2015, 17:46:34) [MSC v.1900 64 bit (AMD64)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import socket
    >>> sock = socket.socket()
    >>> sock.bind(('127.0.0.1', 52380))
    >>> sock.listen(5)
    >>> sock.setblocking(False)
    >>> csock, addr = sock.accept()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "C:\Source\cpython\3.5\lib\socket.py", line 195, in accept
        fd, addr = self._accept()
    BlockingIOError: [WinError 10035] A non-blocking socket operation could not be completed immediately

@vstinner
Copy link
Member

LGTM on Windows 7:

I just compiled Python default (3.6) on Windows 8.1 with Visual Studio 2015: I confirm, the example of the original message now works as expected (raise "BlockingIOError: [WinError 10035] A non-blocking socket operation could not be completed immediately").

It still lacks an unit test.

@larryhastings
Copy link
Contributor

If this is now fixed but still needs a unit test, can I change it from "release blocker" to "high" priority?

@vstinner vstinner changed the title 3.5.0b3 Windows accept() on unready non-blocking socket raises PermissionError 3.5.0b3 Windows accept() on unready non-blocking socket raises PermissionError [now need unit test] Aug 19, 2015
@larryhastings
Copy link
Contributor

3.4 is now EOL, so the 3.4regression tag goes away too.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants