epoll_wait is not interruptable

[micheleselea]

If you have a http keepalive connetion (for example 60 seconds timeout) and you close your application, even if stopAll (true) httpserver close connection the hasMoreRequests() will remain in waiting on epoll_wait

I remember that on windows the only way to terminate epoll_wait was to use
PostQueuedCompletionStatus(epollfd, 0, 0, NULL);

[micheleselea]

The problem is about PollSet has a wakeUp function, SocketImpl::poll dont

[micheleselea]

I think that probably the less effort change in that function to let the close exit as soon as possible is to use a "micropolling" of epoll_wait (for example of 250ms) and wait for the timeout the user want

Anyway I think that probably using epoll_wait in a socket is not optimized, it's ok and understandable in a PollSet but in a socket probably we should use select or poll standard

[micheleselea]

Probably is not the only problem that make http server close slow, I think is something about shutdown of secure socket also, I don't know probably the mutex. I'm digging because I did not notice that in version 14.x

[micheleselea]

Ok I think the most of the time is spent on

int SecureSocketImpl::shutdown()
{
	if (_pSSL)
	{
		UnLockT l(_mutex);  <----here

I have to understand why, because that mutex was not there in my preview lib version

[micheleselea]

OK I found the implementation between shutdown new and old is different.
The old implementation (note the _bidirectShutdown)

void SecureSocketImpl::shutdown()
{
	if (_pSSL)
	{
        // Don't shut down the socket more than once.
        int shutdownState = SSL_get_shutdown(_pSSL);
        bool shutdownSent = (shutdownState & SSL_SENT_SHUTDOWN) == SSL_SENT_SHUTDOWN;
        if (!shutdownSent)
        {
			// A proper clean shutdown would require us to
			// retry the shutdown if we get a zero return
			// value, until SSL_shutdown() returns 1.
			// However, this will lead to problems with
			// most web browsers, so we just set the shutdown
			// flag by calling SSL_shutdown() once and be
			// done with it.
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
			int rc = 0;
			if (!_bidirectShutdown)
				rc = SSL_shutdown(_pSSL);
			else
			{
				Poco::Timespan recvTimeout = _pSocket->getReceiveTimeout();
				Poco::Timespan pollTimeout(0, 100000);
				Poco::Timestamp tsNow;
				do
				{
					rc = SSL_shutdown(_pSSL);
					if (rc == 1) break;
					if (rc < 0)
					{
						int err = SSL_get_error(_pSSL, rc);
						if (err == SSL_ERROR_WANT_READ)
							_pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ);
						else if (err == SSL_ERROR_WANT_WRITE)
							_pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_WRITE);
						else
						{
							int socketError = SocketImpl::lastError();
							long lastError = ERR_get_error();
							if ((err == SSL_ERROR_SSL) && (socketError == 0) && (lastError == 0x0A000123))
								rc = 0;
							break;
						}
					}
					else _pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ);
				} while (!tsNow.isElapsed(recvTimeout.totalMicroseconds()));
			}
#else
			int rc = SSL_shutdown(_pSSL);
#endif
			if (rc < 0) handleError(rc);
			if (_pSocket->getBlocking())
			{
				_pSocket->shutdown();
			}
		}
	}
}

new

int SecureSocketImpl::shutdown()
{
	if (_pSSL)
	{
		UnLockT l(_mutex);

		int shutdownState = ::SSL_get_shutdown(_pSSL);
		bool shutdownSent = (shutdownState & SSL_SENT_SHUTDOWN) == SSL_SENT_SHUTDOWN;
		if (!shutdownSent)
		{
			int rc = 0;
			if (_pSocket->getBlocking())
			{
				// For blocking sockets, perform bidirectional shutdown
				// to ensure all data is received by the peer before closing.
				Poco::Timespan recvTimeout = _pSocket->getReceiveTimeout();
				Poco::Timespan pollTimeout(0, 100000);
				Poco::Timestamp tsStart;
				do
				{
					rc = ::SSL_shutdown(_pSSL);
					if (rc == 1) break;
					if (rc < 0)
					{
						int err = ::SSL_get_error(_pSSL, rc);
						if (err == SSL_ERROR_WANT_READ)
							_pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ);
						else if (err == SSL_ERROR_WANT_WRITE)
							_pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_WRITE);
						else
							break;
					}
					else
					{
						_pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ);
					}
				} while (!tsStart.isElapsed(recvTimeout.totalMicroseconds()));
				if (rc < 0)
				{
					rc = handleError(rc);
				}
			}
			else
			{
				// For non-blocking sockets, call SSL_shutdown() once.
				rc = ::SSL_shutdown(_pSSL);
				if (rc < 0)
				{
					if (SocketImpl::lastError() == POCO_EWOULDBLOCK)
						rc = SecureStreamSocket::ERR_SSL_WOULD_BLOCK;
					else
						rc = handleError(rc);
				}
			}

			l.unlock();

			if (rc >= 0)
			{
				_pSocket->shutdownSend();
			}
			return rc;
		}
		else
		{
			return (shutdownState & SSL_RECEIVED_SHUTDOWN) == SSL_RECEIVED_SHUTDOWN;
		}
	}
	return 1;
}

_bidirectShutdown is gone but I think it's bad, because you need that kind of bidirectional close only in specific scenario, for example in FTPS datasocket where client connect without sending any data. In all other scenario isa safe and faster closing without loop.
I'm going to reintegrate the _bidirectShutdown variable to check it

[micheleselea]

Ok after correcting the shutdown method like in the prev implementation and usinc epoll_wait "mico polling" I have the system exit without problem. I think anyway using epoll_wait inside poll for just poll 1 socket is not the right solution, even if understandable for portable reason linux/windows

[matejk]

Thanks for the analysis. These are two separate problems:

1. SecureSocketImpl::shutdown(): up to 1.13.x the bidirectional wait was skipped once the connection had received application data (_bidirectShutdown). 1.14.0 (NetSSL_OpenSSL and NetSSL_Win: non-blocking support, shutdown behavior fix #4787) removed the wait, causing SecureSocketImpl::shutdown() regression since 1.14.0 #4883; the SecureSocketImpl::shutdown() regression since 1.14.0 #4883 fix in 1.15.2 (fix(Crypto,NetSSL): OpenSSL fixes, version macro cleanup, dead code removal #5246) restored it for all blocking sockets, so every HTTPS server connection now waits up to the receive timeout (60 s default) for the peer's close_notify. Fix: restore the received-data heuristic. SecureSocketImpl::shutdown() regression since 1.14.0 #4883 stays fixed (the FTPS upload socket never reads); non-blocking sockets keep the single SSL_shutdown() call.

2. SocketImpl::poll(): confirmed, Windows-specific. wepoll's epoll_wait() is not woken by closesocket(), which onServerStopped() relies on (same as HTTPServerConnection::onServerStopped does not halt poll #3985); on Linux shutdown() is used and does wake epoll. Fix: single-socket poll() will use select() on Windows and poll() on POSIX; epoll/wepoll stays in PollSet, which has wakeUp(). Also removes the per-call epoll overhead (Why does epoll_close get executed every time when SocketImpl::poll is called? #4918).

Both fixes are planned for 1.16, with regression tests.