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

ClosedResourceError vs. BrokenResourceError is sometimes backend-dependent, and is sometimes not raised at all (in favor of a non-AnyIO exception type) #671

Open
2 tasks done
gschaffner opened this issue Jan 17, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@gschaffner
Copy link
Collaborator

gschaffner commented Jan 17, 2024

Things to check first

  • I have searched the existing issues and didn't find my bug already reported there

  • I have checked that my bug is still present in the latest release

AnyIO version

main

Python version

CPython 3.12.1

What happened?

see the title. here are some reproducers for some issues that i noticed while working on a fix for #669: c6f0334...b6576ae

the main question that these tests raise is: if a send stream is in a Broken state (i.e. our side of the connection learned that it was closed by the peer, or otherwise broken due to something external) and then is explicitly aclose()d by our side, should subsequent calls raise BrokenResourceError or should they raise ClosedResourceError? i.e. if you aclose() a Broken send stream, does that "clear" its Brokenness and convert it to just being Closed?

the documentation does not seem to discuss what should happen if the conditions for ClosedResourceError and BrokenResourceError are both met. i am not sure what the behavior here is intended to be (or if it's intended to be undefined behavior?), as currently different streams do different things in this situation, with the behavior sometimes also varying between backends.

my initial intuition was that the intended behavior was to give precedence to raise BrokenResourceError over raise ClosedResourceError (this choice prevents the stream from changing from Broken to Closed). I thought this becuase this behavior looks like it was rather explicitly chosen when implementing the "important" asyncio-backend streams (TCP and UDP): they explicitly do not set self._closed if they are already closing due to an external cause:

  • SocketStream
    async def aclose(self) -> None:
    if not self._transport.is_closing():
    self._closed = True
    try:
    self._transport.write_eof()
    except OSError:
    pass
    self._transport.close()
    await sleep(0)
    self._transport.abort()
  • UDPStream
    async def aclose(self) -> None:
    if not self._transport.is_closing():
    self._closed = True
    self._transport.close()

so I started to implement it: here is most of an implementation of that behavior: c6f0334...1be403d 1

however, MemoryObjectSendStream is also an "important" stream and it has the opposite behavior, even on the asyncio backend.

How can we reproduce the bug?

see above

Footnotes

  1. note: github shows these commits out of order as it's sorting based on time rather than doing a correct topological sort, so it may be easier to look at these locally, in order.

@gschaffner gschaffner added the bug Something isn't working label Jan 17, 2024
@gschaffner gschaffner changed the title ClosedResourceError vs. BrokenResourceError is sometimes backend-dependent, and is sometimes not raised at all (in favor of a third, wrong exception type) ClosedResourceError vs. BrokenResourceError is sometimes backend-dependent, and is sometimes not raised at all (in favor of a non-AnyIO exception type) Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant