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

TelnetWriter.is_closing() raises AttributeError after TelnetWriter.close() was called. #62

Closed
jonathanslenders opened this issue Feb 13, 2023 · 2 comments

Comments

@jonathanslenders
Copy link

jonathanslenders commented Feb 13, 2023

Hi @jquast, thanks for telnetlib3!

It looks like calling TelnetWriter.close() sets TelnetWriter._protocol to None.
Because of this, calling TelnetWriter.is_closing() after this raises an AttributeError.

Traceback (most recent call last):
  ...
  File "/home/jonathan/git/project/project/src/project/pty_stream/telnet.py", line 122, in ...
    if not instance.is_closing():
  File "/home/jonathan/git/project/project/src/project/pty_stream/telnet.py", line 161, in is_closing
    result: bool = self.stream_writer.is_closing()
  File "/home/jonathan/.pyenv/versions/3.9.13/lib/python3.9/asyncio/streams.py", line 356, in is_closing
    return self._transport.is_closing()
AttributeError: 'NoneType' object has no attribute 'is_closing'

According to the Python docs, is_closing() should return True after close() was called. It should not raise an AttributeError: https://docs.python.org/3/library/asyncio-stream.html#asyncio.StreamWriter.is_closing

Return True if the stream is closed or in the process of being closed.

What I don't understand is the need here to break circular refs. Asyncio's StreamWriter doesn't do it either. Changing the type of a field from the base class from protocols.BaseProtocol to Optional[protocols.BaseProtocol] is a Liskov violation.

edit:
Similar, .wait_closed() can fail with an AttributeError too:

  ...
  File "/home/jonathan/git/radkit/project/.../telnet.py", line 128, in create_acm
    await instance.stream_writer.wait_closed()
  File "/home/jonathan/.pyenv/versions/3.9.13/lib/python3.9/asyncio/streams.py", line 359, in wait_closed
    await self._protocol._get_close_waiter(self)
AttributeError: 'NoneType' object has no attribute '_get_close_waiter'
jquast added a commit that referenced this issue Mar 28, 2023
jquast added a commit that referenced this issue Mar 28, 2023
@jquast
Copy link
Owner

jquast commented Mar 28, 2023

I have addressed this in cecceb8, related, #47 that we should not derive from StreamWriter.

The "circular reference " problem is with the undocumented _waiter_closed and _waiter_connected futures, which are used for tests, they are setting future value of "self" to workaround the difficulty of getting a handle on these things in unit tests, I hope to provide basic general purpose or several specific awaitable methods, like "wait_for_naws()", written about here https://github.com/jquast/telnetlib3/blob/master/DESIGN.rst#wait_for_negotiation, so that I can remove those things and special code in close() method entirely.

@jquast jquast mentioned this issue Mar 28, 2023
@jquast
Copy link
Owner

jquast commented Mar 29, 2023

Closed by release of 2.0.1 https://telnetlib3.readthedocs.io/en/master/history.html#history

Thanks again @jonathanslenders I'm a big fan of your work, may you have a great summer and write some great new codes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants