#12049 Use valid "clock" in TLSMemoryBIOFactory

[ddzialak]

Scope and purpose

Fixes #12048

Without this change, using IOCPReactor with TLS and Keep-Alive
will cause that response from server will be cached and not sent to the client.

Contributor Checklist:

This process applies to all pull requests - no matter how small.
Have a look at our developer documentation before submitting your Pull Request.

Below is a non-exhaustive list (as a reminder):

• The title of the PR should describe the changes and starts with the associated issue number, like “#9782 Remove twisted.news. #1234 Brief description”.
• A release notes news fragment file was create in src/twisted/newsfragments/ (see: Release notes fragments docs.)
• The automated tests were updated.
• Once all checks are green, request a review by leaving a comment that contains exactly the string please review.
  Our bot will trigger the review process, by applying the pending review label
  and requesting a review from the Twisted dev team.

[ddzialak]

please review

[adiroiban]

Thanks for the PR.

I think that we have a bigger issue here.
If no clock is passed to the TLS factory, it should just use the default reactor.

from twisted.internet import reactor

---

Do you have multiple reactors installed on your application ?

If somewhere later from your code you call from twisted.internet import reactor do you get the IOCP reactor of the select reactor?

[adiroiban]

I think that the fix and the root cause is more generic. This not only about TLS and keep-alive.

I still trying to understand what is going in here... but it looks like in the trunk branch TLSMemoryBIOFactory is using the default reactor... and somehow, that reactor is not the same reactor that you are using for your HTTP application.

I am not sure.

I self contained exemple would help a lot to find the root cause of this issue.

Thanks

[ddzialak]

Probably it is a consequence of that default:
https://github.com/twisted/twisted/blob/trunk/src/twisted/internet/default.py#L49

In our code we import only one reactor:

if is_windows:
    from twisted.internet.iocpreactor.reactor import IOCPReactor as Reactor
elif is_cygwin:
    # Beware, select is currently limited to 512 sockets, see:
    # https://bugs.python.org/issue28708
    from twisted.internet.selectreactor import SelectReactor as Reactor
else:
    from twisted.internet.epollreactor import EPollReactor as Reactor  # type: ignore

BUT we have defined our class:

from twisted.web.server import NOT_DONE_YET, Site

class _SfSite(Site):

and adding raising exception in selectreactor give traceback:

  File "c:\tmp\darek-testing\repo\starfish\starfish\src\sfutils\service\twisted_app_server.py", line 230, in handle_requests
    site = _SfSite(
           ^^^^^^^^
  File "c:\tmp\darek-testing\repo\starfish\starfish\src\sfutils\service\twisted_app_server.py", line 153, in __init__
    super().__init__(resource, **kwargs)
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\web\server.py", line 814, in __init__
    super().__init__(*args, **kwargs)
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\web\http.py", line 3226, in __init__
    from twisted.internet import reactor
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\internet\reactor.py", line 38, in <module>
    from twisted.internet import default
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\internet\default.py", line 55, in <module>
    install = _getInstallFunction(platform)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\internet\default.py", line 49, in _getInstallFunction
    from twisted.internet.selectreactor import install
  File "C:\tmp\darek-testing\venv\Lib\site-packages\twisted\internet\selectreactor.py", line 199, in <module>
    raise AssertionError("OPS")

[adiroiban]

it's ok to have a custom site.

Just to make sure
So, for the reactor, I guess that you have something like this at the very start of your app

from twisted.internet import iocpreactor
iocpreactor.install()

# Then everywhere else:
from twisted.internet import reactor

The documentation is here ... not sure if the docs are good enough

https://docs.twisted.org/en/stable/core/howto/choosing-reactor.html#core-howto-choosing-reactor-win32-iocp

---

We also need an automated test to make sure we will never reintroduce this bug, ever again :)

[ddzialak]

We don't have iocpreactor.install() because we have our custom Reactor that is a subclass of IOCPReactor and it wasn't clear that "install" is needed. Anyway, as I see these "install()" functions just create reactor instance and invoke main.installReactor(reactor).
I've verified that after invoking "main.installReactor(reactor)" problem also disappeared so I will go for that, closing that issue.

[adiroiban]

Happy to hear this was fixed.

Intall part is required, even for custome reactors.

This is what makes a singleton for the default reactor.