Skip to content

Commit

Permalink
Standardize the names of wait_* and notify_closing across platforms
Browse files Browse the repository at this point in the history
  • Loading branch information
njsmith committed Jun 20, 2019
1 parent bfea5ec commit 78027f9
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 151 deletions.
10 changes: 5 additions & 5 deletions docs/source/history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,9 @@ Features
(`#56 <https://github.com/python-trio/trio/issues/56>`__)
- New and improved signal catching API: :func:`open_signal_receiver`. (`#354
<https://github.com/python-trio/trio/issues/354>`__)
- The low level :func:`trio.hazmat.wait_socket_readable`,
:func:`~trio.hazmat.wait_socket_writable`, and
:func:`~trio.hazmat.notify_socket_close` now work on bare socket descriptors,
- The low level ``trio.hazmat.wait_socket_readable``,
``wait_socket_writable``, and
``notify_socket_close`` now work on bare socket descriptors,
instead of requiring a :func:`socket.socket` object. (`#400
<https://github.com/python-trio/trio/issues/400>`__)
- If you're using :func:`trio.hazmat.wait_task_rescheduled` and other low-level
Expand Down Expand Up @@ -303,8 +303,8 @@ Features
:exc:`ClosedResourceError`. ``ClosedStreamError`` and ``ClosedListenerError``
are now aliases for :exc:`ClosedResourceError`, and deprecated. For this to
work, Trio needs to know when a resource has been closed. To facilitate this,
new functions have been added: :func:`trio.hazmat.notify_fd_close` and
:func:`trio.hazmat.notify_socket_close`. If you're using Trio's built-in
new functions have been added: ``trio.hazmat.notify_fd_close`` and
``trio.hazmat.notify_socket_close``. If you're using Trio's built-in
wrappers like :class:`~trio.SocketStream` or :mod:`trio.socket`, then you don't
need to worry about this, but if you're using the low-level functions like
:func:`trio.hazmat.wait_readable`, you should make sure to call these
Expand Down
109 changes: 25 additions & 84 deletions docs/source/reference-hazmat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -124,113 +124,54 @@ Universally available API

All environments provide the following functions:

.. function:: wait_socket_readable(sock)
.. function:: wait_readable(obj)
:async:

Block until the given :func:`socket.socket` object is readable.
Block until the kernel reports that the given object is readable.

On Unix systems, sockets are fds, and this is identical to
:func:`wait_readable`. On Windows, ``SOCKET`` handles and fds are
different, and this works on ``SOCKET`` handles or Python socket
objects.
On Unix systems, ``obj`` must either be an integer file descriptor,
or else an object with a ``.fileno()`` method which returns an
integer file descriptor. Any kind of file descriptor can be passed,
though the exact semantics will depend on your kernel. For example,
this probably won't do anything useful for on-disk files.

:raises trio.BusyResourceError:
if another task is already waiting for the given socket to
become readable.

.. function:: wait_socket_writable(sock)
:async:

Block until the given :func:`socket.socket` object is writable.

On Unix systems, sockets are fds, and this is identical to
:func:`wait_writable`. On Windows, ``SOCKET`` handles and fds are
different, and this works on ``SOCKET`` handles or Python socket
objects.
On Windows systems, ``obj`` must either be an integer ``SOCKET``
handle, or else an object with a ``.fileno()`` method which returns
an integer ``SOCKET`` handle. File descriptors aren't supported,
and neither are handles that refer to anything besides a
``SOCKET``.

:raises trio.BusyResourceError:
if another task is already waiting for the given socket to
become writable.
:raises trio.ClosedResourceError:
if another task calls :func:`notify_socket_close` while this
function is still working.


.. function:: notify_socket_close(sock)

Notifies Trio's internal I/O machinery that you are about to close
a socket.

This causes any operations currently waiting for this socket to
immediately raise :exc:`~trio.ClosedResourceError`.

This does *not* actually close the socket. Generally when closing a
socket, you should first call this function, and then close the
socket.

On Unix systems, sockets are fds, and this is identical to
:func:`notify_fd_close`. On Windows, ``SOCKET`` handles and fds are
different, and this works on ``SOCKET`` handles or Python socket
objects.


Unix-specific API
-----------------

Unix-like systems provide the following functions:

.. function:: wait_readable(fd)
:async:

Block until the given file descriptor is readable.

.. warning::

This is "readable" according to the operating system's
definition of readable. In particular, it probably won't tell
you anything useful for on-disk files.

:arg fd:
integer file descriptor, or else an object with a ``fileno()`` method
:raises trio.BusyResourceError:
if another task is already waiting for the given fd to
become readable.
:raises trio.ClosedResourceError:
if another task calls :func:`notify_fd_close` while this
if another task calls :func:`notify_closing` while this
function is still working.


.. function:: wait_writable(fd)
.. function:: wait_writable(obj)
:async:

Block until the given file descriptor is writable.

.. warning::
Block until the kernel reports that the given object is writable.

This is "writable" according to the operating system's
definition of writable. In particular, it probably won't tell
you anything useful for on-disk files.
See `wait_readable` for the definition of ``obj``.

:arg fd:
integer file descriptor, or else an object with a ``fileno()`` method
:raises trio.BusyResourceError:
if another task is already waiting for the given fd to
if another task is already waiting for the given socket to
become writable.
:raises trio.ClosedResourceError:
if another task calls :func:`notify_fd_close` while this
if another task calls :func:`notify_closing` while this
function is still working.

.. function:: notify_fd_close(fd)

Notifies Trio's internal I/O machinery that you are about to close
a file descriptor.
.. function:: notify_closing(obj)

This causes any operations currently waiting for this file
descriptor to immediately raise :exc:`~trio.ClosedResourceError`.
Call this before closing a file descriptor or ``SOCKET`` handle
that another task might be waiting on. This will cause any
`wait_readable` or `wait_writable` calls to immediately raise
`~trio.ClosedResourceError`.

This does *not* actually close the file descriptor. Generally when
closing a file descriptor, you should first call this function, and
then actually close it.
This doesn't actually close the object – you still have to do that
yourself afterwards.


Kqueue-specific API
Expand Down
6 changes: 6 additions & 0 deletions newsfragments/878.removal.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Give up on trying to have different low-level waiting APIs on Unix and
Windows. All platforms now have `trio.hazmat.wait_readable`,
`trio.hazmat.wait_writable`, and `trio.hazmat.notify_closing`. The old
platform-specific synonyms ``wait_socket_*``,
``notify_socket_closing``, and ``notify_fd_closing`` have been
deprecated.
30 changes: 28 additions & 2 deletions trio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,43 @@
run_sync_in_thread,
"0.12.0",
issue=810,
instead=run_sync_in_thread,
),
"current_default_worker_thread_limiter":
_deprecate.DeprecatedAttribute(
current_default_thread_limiter,
"0.12.0",
issue=810,
instead=current_default_thread_limiter,
),
}

_deprecate.enable_attribute_deprecations(hazmat.__name__)
hazmat.__deprecated_attributes__ = {
"wait_socket_readable":
_deprecate.DeprecatedAttribute(
hazmat.wait_readable,
"0.12.0",
issue=878,
),
"wait_socket_writable":
_deprecate.DeprecatedAttribute(
hazmat.wait_writable,
"0.12.0",
issue=878,
),
"notify_socket_close":
_deprecate.DeprecatedAttribute(
hazmat.notify_closing,
"0.12.0",
issue=878,
),
"notify_fd_close":
_deprecate.DeprecatedAttribute(
hazmat.notify_closing,
"0.12.0",
issue=878,
),
}

# Having the public path in .__module__ attributes is important for:
# - exception names in printed tracebacks
# - sphinx :show-inheritance:
Expand Down
5 changes: 0 additions & 5 deletions trio/_core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,3 @@ def _public(fn):
from ._unbounded_queue import UnboundedQueue

from ._local import RunVar

if hasattr(_run, "wait_readable"):
wait_socket_readable = wait_readable
wait_socket_writable = wait_writable
notify_socket_close = notify_fd_close
2 changes: 1 addition & 1 deletion trio/_core/_io_epoll.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ async def wait_writable(self, fd):
await self._epoll_wait(fd, "write_task")

@_public
def notify_fd_close(self, fd):
def notify_closing(self, fd):
if not isinstance(fd, int):
fd = fd.fileno()
if fd not in self._registered:
Expand Down
2 changes: 1 addition & 1 deletion trio/_core/_io_kqueue.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ async def wait_writable(self, fd):
await self._wait_common(fd, select.KQ_FILTER_WRITE)

@_public
def notify_fd_close(self, fd):
def notify_closing(self, fd):
if not isinstance(fd, int):
fd = fd.fileno()

Expand Down
6 changes: 3 additions & 3 deletions trio/_core/_io_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,15 @@ def abort(_):
await _core.wait_task_rescheduled(abort)

@_public
async def wait_socket_readable(self, sock):
async def wait_readable(self, sock):
await self._wait_socket("read", sock)

@_public
async def wait_socket_writable(self, sock):
async def wait_writable(self, sock):
await self._wait_socket("write", sock)

@_public
def notify_socket_close(self, sock):
def notify_closing(self, sock):
if not isinstance(sock, int):
sock = sock.fileno()
for mode in ["read", "write"]:
Expand Down
2 changes: 1 addition & 1 deletion trio/_core/_wakeup_socketpair.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def wakeup_thread_and_signal_safe(self):
pass

async def wait_woken(self):
await _core.wait_socket_readable(self.wakeup_sock)
await _core.wait_readable(self.wakeup_sock)
self.drain()

def drain(self):
Expand Down
32 changes: 12 additions & 20 deletions trio/_core/tests/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from ... import _core
from ...testing import wait_all_tasks_blocked, Sequencer, assert_checkpoints
import trio

# Cross-platform tests for IO handling

Expand Down Expand Up @@ -45,36 +46,27 @@ def fileno_wrapper(fileobj):
return fileno_wrapper


wait_readable_options = [_core.wait_socket_readable]
wait_writable_options = [_core.wait_socket_writable]
notify_close_options = [_core.notify_socket_close]

# We aren't testing the _fd_ versions, because they're the same as the socket
# ones. But if they ever stop being the same we should notice and add tests!
if hasattr(_core, "wait_readable"):
assert _core.wait_socket_readable is _core.wait_readable
if hasattr(_core, "wait_writable"):
assert _core.wait_socket_writable is _core.wait_writable
if hasattr(_core, "notify_fd_close"):
assert _core.notify_socket_close is _core.notify_fd_close
wait_readable_options = [trio.hazmat.wait_readable]
wait_writable_options = [trio.hazmat.wait_writable]
notify_closing_options = [trio.hazmat.notify_closing]

for options_list in [
wait_readable_options, wait_writable_options, notify_close_options
wait_readable_options, wait_writable_options, notify_closing_options
]:
options_list += [using_fileno(f) for f in options_list]

# Decorators that feed in different settings for wait_readable / wait_writable
# / notify_close.
# / notify_closing.
# Note that if you use all three decorators on the same test, it will run all
# N**4 *combinations*
# N**3 *combinations*
read_socket_test = pytest.mark.parametrize(
"wait_readable", wait_readable_options, ids=lambda fn: fn.__name__
)
write_socket_test = pytest.mark.parametrize(
"wait_writable", wait_writable_options, ids=lambda fn: fn.__name__
)
notify_close_test = pytest.mark.parametrize(
"notify_close", notify_close_options, ids=lambda fn: fn.__name__
notify_closing_test = pytest.mark.parametrize(
"notify_closing", notify_closing_options, ids=lambda fn: fn.__name__
)


Expand Down Expand Up @@ -177,9 +169,9 @@ async def test_double_write(socketpair, wait_writable):

@read_socket_test
@write_socket_test
@notify_close_test
@notify_closing_test
async def test_interrupted_by_close(
socketpair, wait_readable, wait_writable, notify_close
socketpair, wait_readable, wait_writable, notify_closing
):
a, b = socketpair

Expand All @@ -197,7 +189,7 @@ async def writer():
nursery.start_soon(reader)
nursery.start_soon(writer)
await wait_all_tasks_blocked()
notify_close(a)
notify_closing(a)


@read_socket_test
Expand Down
Loading

0 comments on commit 78027f9

Please sign in to comment.