[issue32038] Add API to intercept socket.close()

2018-05-28 Thread Yury Selivanov
Yury Selivanov added the comment: Closing this one. Using private socket.socket APIs works fine for uvloop/asyncio. -- resolution: -> rejected stage: -> resolved status: open -> closed type: -> enhancement ___ Python tracker

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > Note that a guard on socket objects can only solve part of the problem: in > the case where i've seen this bug, it was with raw file descriptors from > libcurl, and there's nothing python can do about that because there are no > (python) socket objects. We

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Ben Darnell
Ben Darnell added the comment: Note that a guard on socket objects can only solve part of the problem: in the case where i've seen this bug, it was with raw file descriptors from libcurl, and there's nothing python can do about that because there are no (python) socket objects. -- _

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > OK, then maybe socket objects should get a dup() method that just increments the ref counter, and that would be the public API you're looking for? "dup()" returns a new socket object, but here we only want to protect the original one. Maybe just 'sock.inc_i

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Guido van Rossum
Guido van Rossum added the comment: OK, then maybe socket objects should get a dup() method that just increments the ref counter, and that would be the public API you're looking for? -- ___ Python tracker ___

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > Perhaps you can just dup() the socket? That's what the ref counter is for > IIRC. I already dup them for loop.create_server(sock=sock) and loop.create_connection(sock=sock) cases. Duping for sock_recv & friends will add too much overhead (they are relativ

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread STINNER Victor
STINNER Victor added the comment: > Perhaps you can just dup() the socket? That's what the ref counter is for > IIRC. Sure. That's another option. But for asyncio with a lot of concurrent users, I'm not sure that it's ok to create a temporary file descriptor, since there is a risk of hitting

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Guido van Rossum
Guido van Rossum added the comment: Perhaps you can just dup() the socket? That's what the ref counter is for IIRC. -- ___ Python tracker ___ __

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > Hmm... _decref_socketios() is not really a public API. I think it would be > nice to have an official way to deal with this, and a socket close callback > sounds reasonable to me. We can just make it public (after some renames) :) My first idea was to ad

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread STINNER Victor
STINNER Victor added the comment: > Hmm... _decref_socketios() is not really a public API. I think it would be > nice to have an official way to deal with this, and a socket close callback > sounds reasonable to me. I dislike the idea of an event when a socket is closed. It doesn't prevent a

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Antoine Pitrou
Antoine Pitrou added the comment: Hmm... _decref_socketios() is not really a public API. I think it would be nice to have an official way to deal with this, and a socket close callback sounds reasonable to me. -- ___ Python tracker

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > Oh, sock._decref_socketios() must be uesd here to really close the socket if > it was closed in the meanwhile: There's also 'sock._closed' attribute that sock.close() and sock. _decref_socketios() interact with.. Alright, let me play with this in uvloop.

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Change by Yury Selivanov : -- Removed message: https://bugs.python.org/msg306374 ___ Python tracker ___ ___ Python-bugs-list mailing

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > Oh, sock._decref_socketios() must be uesd here to really close the socket if > it was closed in the meanwhile: For it to work correctly, socket objects should be initialized with "self._io_refs = 1". -- ___ Pyth

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread STINNER Victor
STINNER Victor added the comment: >From my example: "finally: sock._io_refs -= 1" Oh, sock._decref_socketios() must be uesd here to really close the socket if it was closed in the meanwhile: def _decref_socketios(self): if self._io_refs > 0: self._io_refs -= 1

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > The socket.socket (Python type) has a private _io_refs counter. close() does > nothing until _io_refs reachs zero. I didn't know about this. Looks like I can use '_io_refs' to fix some problems in uvloop, thanks Victor! The only problem with '_io_refs' an

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread STINNER Victor
STINNER Victor added the comment: The socket.socket (Python type) has a private _io_refs counter. close() does nothing until _io_refs reachs zero. Maybe we can develop an API to temporary increase the _io_refs counter to prevent a real close? Pseudo-code: @contextlib.contextmanager def dont

[issue32038] Add API to intercept socket.close()

2017-11-16 Thread Yury Selivanov
Yury Selivanov added the comment: > It's worse than a resource leak - the same file descriptor number could be > reused for a different file/socket, and then depending on the selector in > use, you could see the data from a completely different connection. I actually debugged a bug like this

[issue32038] Add API to intercept socket.close()

2017-11-15 Thread Ben Darnell
Ben Darnell added the comment: It's worse than a resource leak - the same file descriptor number could be reused for a different file/socket, and then depending on the selector in use, you could see the data from a completely different connection. I did see a bug like this years ago (in libc

[issue32038] Add API to intercept socket.close()

2017-11-15 Thread Antoine Pitrou
Antoine Pitrou added the comment: Ok, that sounds reasonable to me. Also cc'ing Ben Darnell (maintainer of Tornado) in case he wants to chime in. -- nosy: +Ben.Darnell ___ Python tracker ___

[issue32038] Add API to intercept socket.close()

2017-11-15 Thread Yury Selivanov
Yury Selivanov added the comment: > Why not instead add `loop.sock_close()`? It wouldn't help if the program calls `socket.close()` somewhere. This can happen easily in big code bases. Ideally (at least I think so) we need to provide a guarantee that the socket object can't be closed at all

[issue32038] Add API to intercept socket.close()

2017-11-15 Thread Antoine Pitrou
Antoine Pitrou added the comment: Why not instead add `loop.sock_close()`? -- ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue32038] Add API to intercept socket.close()

2017-11-15 Thread Yury Selivanov
New submission from Yury Selivanov : asyncio has a few APIs that accept a Python socket object: loop.sock_recv(), loop.sock_accept() etc. asyncio (and event loops such as uvloop) expect that it controls the socket for the duration of the call. However, it cannot prevent the socket object from