Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1251
Thanks! I am going to need some time to review it further. The server
layer should know nothing about the transport which it looks like you
addressed, but similarly the transport layer should know nothing about the
server. There's a new TNonblockingSSLServerSocket class here - this seems to
be combining the responsibility of the transport with that of knowing the
server type is.
Here are a couple things to consider from a quick review:
1. I wonder if you need the Nonblocking socket classes; instead if you
have TNonblockingServer call TSSLSocket::setLibeventSafe(false) after asking
for the socket from the factory - you might not need a Nonblocking factory at
all (a nonblocking socket factory combines server and transport concerns).
2. Is there any reason isLibeventSafe() branches only apply to TSSLSocket?
If the same reasoning applies to TSocket as well, then perhaps that logic moves
into TSocket so TNonblockingServer uses TSocket the same way as TSSLSocket?
I'm curious what the specific "not libevent safe" mechanisms are. I'll
pick that up from further code review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
