Skip to content

Commit baaf77f

Browse files
authored
Clean up UnixNetProcessor entanglements. (#9825)
The NetProcessor class sometimes downcasts itself to a UnixNetProcessor because it implicitly knows that it is a singleton and that UnixNetProcessor is the actual implementation. We can remove this oddity by simply making the relevant NetProcessor operations abstract and moving the implementations to UnixNetProcessor. While we are doing this, we can remove some unused member variables, make createNetAccept protected (only subclasses of UnixNetProcessor should call it), remove unnecessary casting, and add a missing lock to a naVec traversal. Signed-off-by: James Peach <jpeach@apache.org>
1 parent 6411b26 commit baaf77f

12 files changed

+67
-101
lines changed

iocore/net/I_NetProcessor.h

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class NetProcessor : public Processor
134134
@return Action, that can be cancelled to cancel the accept. The
135135
port becomes free immediately.
136136
*/
137-
virtual Action *accept(Continuation *cont, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS);
137+
virtual Action *accept(Continuation *cont, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS) = 0;
138138

139139
/**
140140
Accepts incoming connections on port. Accept connections on port.
@@ -160,8 +160,9 @@ class NetProcessor : public Processor
160160
port becomes free immediately.
161161
162162
*/
163-
virtual Action *main_accept(Continuation *cont, SOCKET listen_socket_in, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS);
164-
virtual void stop_accept();
163+
virtual Action *main_accept(Continuation *cont, SOCKET listen_socket_in, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS) = 0;
164+
165+
virtual void stop_accept() = 0;
165166

166167
/**
167168
Open a NetVConnection for connection oriented I/O. Connects
@@ -181,8 +182,7 @@ class NetProcessor : public Processor
181182
@param options @see NetVCOptions.
182183
183184
*/
184-
185-
Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *options = nullptr);
185+
virtual Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *options = nullptr) = 0;
186186

187187
/**
188188
Initializes the net processor. This must be called before the event threads are started.
@@ -225,15 +225,6 @@ class NetProcessor : public Processor
225225
// noncopyable
226226
NetProcessor(const NetProcessor &) = delete;
227227
NetProcessor &operator=(const NetProcessor &) = delete;
228-
229-
private:
230-
/** @note Not implemented. */
231-
virtual int
232-
stop()
233-
{
234-
ink_release_assert(!"NetProcessor::stop not implemented");
235-
return 1;
236-
}
237228
};
238229

239230
/**

iocore/net/P_QUICNetProcessor_native.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,19 @@ class QUICNetProcessor : public UnixNetProcessor
5757
virtual ~QUICNetProcessor();
5858

5959
void init() override;
60-
virtual int start(int, size_t stacksize) override;
61-
// TODO: refactoring NetProcessor::connect_re and UnixNetProcessor::connect_re_internal
62-
// Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts) override;
63-
Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts);
60+
int start(int, size_t stacksize) override;
6461

65-
virtual NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
66-
virtual NetVConnection *allocate_vc(EThread *t) override;
62+
Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts) override;
63+
64+
NetVConnection *allocate_vc(EThread *t) override;
6765

6866
Action *main_accept(Continuation *cont, SOCKET fd, AcceptOptions const &opt) override;
6967

7068
off_t quicPollCont_offset;
7169

70+
protected:
71+
NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
72+
7273
private:
7374
QUICNetProcessor(const QUICNetProcessor &);
7475
QUICNetProcessor &operator=(const QUICNetProcessor &);

iocore/net/P_QUICNetProcessor_quiche.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,19 @@ class QUICNetProcessor : public UnixNetProcessor
5757
virtual ~QUICNetProcessor();
5858

5959
void init() override;
60-
virtual int start(int, size_t stacksize) override;
61-
// TODO: refactoring NetProcessor::connect_re and UnixNetProcessor::connect_re_internal
62-
// Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts) override;
63-
Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts);
60+
int start(int, size_t stacksize) override;
6461

65-
virtual NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
66-
virtual NetVConnection *allocate_vc(EThread *t) override;
62+
Action *connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts) override;
63+
64+
NetVConnection *allocate_vc(EThread *t) override;
6765

6866
Action *main_accept(Continuation *cont, SOCKET fd, AcceptOptions const &opt) override;
6967

7068
off_t quicPollCont_offset;
7169

70+
protected:
71+
NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
72+
7273
private:
7374
QUICNetProcessor(const QUICNetProcessor &);
7475
QUICNetProcessor &operator=(const QUICNetProcessor &);

iocore/net/P_SSLNetProcessor.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,17 @@ struct SSLNetProcessor : public UnixNetProcessor {
5555
public:
5656
int start(int, size_t stacksize) override;
5757

58-
void cleanup();
59-
6058
SSLNetProcessor();
6159
~SSLNetProcessor() override;
6260

63-
//
64-
// Private
65-
//
66-
67-
NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
6861
NetVConnection *allocate_vc(EThread *t) override;
6962

7063
// noncopyable
7164
SSLNetProcessor(const SSLNetProcessor &) = delete;
7265
SSLNetProcessor &operator=(const SSLNetProcessor &) = delete;
66+
67+
protected:
68+
NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt) override;
7369
};
7470

7571
extern SSLNetProcessor ssl_NetProcessor;

iocore/net/P_UnixNetProcessor.h

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,35 +35,29 @@ class UnixNetVConnection;
3535
//
3636
//////////////////////////////////////////////////////////////////
3737
struct UnixNetProcessor : public NetProcessor {
38+
private:
39+
Action *accept_internal(Continuation *cont, int fd, AcceptOptions const &opt);
40+
41+
protected:
42+
virtual NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt);
43+
3844
public:
39-
virtual Action *accept_internal(Continuation *cont, int fd, AcceptOptions const &opt);
45+
Action *accept(Continuation *cont, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS) override;
46+
Action *main_accept(Continuation *cont, SOCKET listen_socket_in, AcceptOptions const &opt = DEFAULT_ACCEPT_OPTIONS) override;
4047

41-
Action *connect_re_internal(Continuation *cont, sockaddr const *target, NetVCOptions *options = nullptr);
42-
Action *connect(Continuation *cont, UnixNetVConnection **vc, sockaddr const *target, NetVCOptions *opt = nullptr);
48+
void stop_accept() override;
4349

44-
virtual NetAccept *createNetAccept(const NetProcessor::AcceptOptions &opt);
50+
Action *connect_re(Continuation *cont, sockaddr const *target, NetVCOptions *options = nullptr) override;
4551
NetVConnection *allocate_vc(EThread *t) override;
4652

4753
void init() override;
4854
void init_socks() override;
4955

50-
Event *accept_thread_event;
51-
5256
// offsets for per thread data structures
5357
off_t netHandler_offset;
5458
off_t pollCont_offset;
55-
56-
// we probably won't need these members
57-
int n_netthreads;
58-
EThread **netthreads;
5959
};
6060

61-
TS_INLINE Action *
62-
NetProcessor::connect_re(Continuation *cont, sockaddr const *addr, NetVCOptions *opts)
63-
{
64-
return static_cast<UnixNetProcessor *>(this)->connect_re_internal(cont, addr, opts);
65-
}
66-
6761
extern UnixNetProcessor unix_netProcessor;
6862

6963
//

iocore/net/QUICNetProcessor.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ QUICNetProcessor::createNetAccept(const NetProcessor::AcceptOptions &opt)
8282
this->_ctable = new QUICConnectionTable(params->connection_table_size());
8383
this->_rtable = new QUICResetTokenTable();
8484
}
85-
return (NetAccept *)new QUICPacketHandlerIn(opt, *this->_ctable, *this->_rtable);
85+
return new QUICPacketHandlerIn(opt, *this->_ctable, *this->_rtable);
8686
}
8787

8888
NetVConnection *

iocore/net/QUICNetProcessor_quiche.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ QUICNetProcessor::createNetAccept(const NetProcessor::AcceptOptions &opt)
109109
QUICConfig::scoped_config params;
110110
this->_ctable = new QUICConnectionTable(params->connection_table_size());
111111
}
112-
return (NetAccept *)new QUICPacketHandlerIn(opt, *this->_ctable, *this->_quiche_config);
112+
return new QUICPacketHandlerIn(opt, *this->_ctable, *this->_quiche_config);
113113
}
114114

115115
NetVConnection *

iocore/net/QUICPacketHandler_quiche.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ QUICPacketHandlerIn::init_accept(EThread *t = nullptr)
184184
Continuation *
185185
QUICPacketHandlerIn::_get_continuation()
186186
{
187-
return static_cast<NetAccept *>(this);
187+
return this;
188188
}
189189

190190
void

iocore/net/SSLNetProcessor.cc

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,6 @@ struct OCSPContinuation : public Continuation {
5252
OCSPContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&OCSPContinuation::mainEvent); }
5353
};
5454

55-
void
56-
SSLNetProcessor::cleanup()
57-
{
58-
}
59-
6055
int
6156
SSLNetProcessor::start(int, size_t stacksize)
6257
{
@@ -92,7 +87,7 @@ SSLNetProcessor::start(int, size_t stacksize)
9287
NetAccept *
9388
SSLNetProcessor::createNetAccept(const NetProcessor::AcceptOptions &opt)
9489
{
95-
return (NetAccept *)new SSLNetAccept(opt);
90+
return new SSLNetAccept(opt);
9691
}
9792

9893
NetVConnection *
@@ -113,7 +108,4 @@ SSLNetProcessor::allocate_vc(EThread *t)
113108

114109
SSLNetProcessor::SSLNetProcessor() {}
115110

116-
SSLNetProcessor::~SSLNetProcessor()
117-
{
118-
cleanup();
119-
}
111+
SSLNetProcessor::~SSLNetProcessor() {}

iocore/net/UnixNetAccept.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ using NetAcceptHandler = int (NetAccept::*)(int, void *);
3030

3131
int NetAccept::accept_till_done = 1;
3232

33-
// we need to protect naVec since it might be accessed
34-
// in different threads at the same time
35-
Ptr<ProxyMutex> naVecMutex;
36-
std::vector<NetAccept *> naVec;
3733
static void
3834
safe_delay(int msec)
3935
{

0 commit comments

Comments
 (0)