epee net tcp server: properly terminate interrupted TCP connection. fixes #8685

[BotoX]

See issue #8685
Specifically the comment/patch by @j-berman #8685 (comment)

monerod would start using 160-180% CPU after a few days of uptime.
I was consistently running into this issue for multiple months.
With this patch monerod has been running for two weeks already, without high cpu usage.

Related/Same issue #8550

[plowsof]

fix commit author (or ask them to PR the fix)

[BotoX]

fix commit author (or ask them to PR the fix)

done.

[iamamyth]

Given the change below, this else if can be folded into the else.

[j-berman]

Repeating my logic for this patch:

I'm thinking until someone actually writes the code to reuse an "interrupted" connection, it makes sense to simply complete the connection termination sequence (and shut down the TCP connection) after the server shuts the SSL stream down. This way the server isn't leaving a connection around in this interrupted state that can't be "un-interrupted."

Adding some more logic to support this reasoning... once a connection has entered the interrupted state, it will stop reading data received over the socket thanks to this section:

monero/contrib/epee/include/net/abstract_tcp_server2.inl

Lines 412 to 420 in c5d10a4

	if (m_state.status == status_t::INTERRUPTED)
	on_interrupted();
	else if (m_state.status == status_t::TERMINATING)
	on_terminating();
	else if (!success)
	interrupt();
	else {
	start_read();
	}

When the connection is in the interrupted state, it falls into the first if, preventing start_read from getting called in the else. start_read is what calls async_read_some to read the next chunk of data received over the socket. Thus, the interrupted state prevents the server from reading more data received over the socket.

As written, I don't see why the server should keep an interrupted connection around.

[vtnerd]

As written, I don't see why the server should keep an interrupted connection around.

What is the purpose of the state? At a glance I don't see where the state/status can become INTERRUPTED.

Whoops, funky search before, I see it now. But still, what is the purpose of the state?

[j-berman]

From what I recall in discussions with the author, their intent behind the connection's interrupted status was to allow the server to reuse a connection that hasn't been terminated yet. But the author didn't actually implement the code to reuse an interrupted connection (I don't see a way for a connection's status to go from INTERRUPTED to anything but TERMINATING or WASTED). I believe they intended to implement the ability to reuse the underlying TCP connection in a future PR building off this interrupted status.

More behind my thinking on this patch is here: #8685 (comment)

[vtnerd]

@j-berman its tempting to slowly remove the interrupted state completely - it seems to complicated for our use cases.