subclasses of telnetlib.Telnet may crash when used as context manager about cpython

[serhiy-storchaka]

How can it call Telnet.__exit__ before finishing Telnet.__init__?

[NichtJens]

How can it call Telnet.__exit__ before finishing Telnet.__init__?

I am confused. I think I gave an example for this in the report. The short answer is "subclasses". Here is the relevant part of the example:

class AliasTelnet(telnetlib.Telnet):

    def __init__(self, alias):
        host = ALIAS_TO_HOST.get(alias)
        if not host:
            raise ValueError(f'cannot map alias "{alias}" to host name')

        super().__init__(host)

Raising something in AliasTelnet.__init__ will call Telnet.__exit__ on its way out.

[serhiy-storchaka]

Why does it call __exit__ on its way out? Normally, __exit__ is only called after __enter__, implicitly in the with statement, and you need already created object for this. __init__ should finish successfully before you can use the object as a context manager.

[NichtJens]

I am sorry, I misread the traceback. It does not happen in __exit__ but in __del__. It's actually saying that in the traceback:

Exception ignored in: <function Telnet.__del__ at 0x7faeb8656ee0>

[NichtJens]

I updated the report accordingly.

[serhiy-storchaka]

Indeed, this is a bug. Your second version is more preferable because it is simpler. There is already a check for self.sock.

Note that the telnetlib module was deprecated in Python 3.11 and removed in 3.13.

3.12 is the only version that will get the fix. 3.11 and older only accept security fixes.

[NichtJens]

Your second version is more preferable because it is simpler. There is already a check for self.sock.

I would have argued the opposite. In particular since you felt it was needed to add the comment:

# for __del__()

So, solving the problem where it happens (reading sock = self.sock) feels cleaner.

But both are fine ...

Note that the telnetlib module was deprecated in Python 3.11 and removed in 3.13.

3.12 is the only version that will get the fix. 3.11 and older only accept security fixes.

Yes, I figured that -- see last paragraph in the report :)