Skip to content
Permalink
Browse files

Fix 3.1.3 crash on SELECT query over HTTP interface

  • Loading branch information
klirichek committed Oct 18, 2019
1 parent fac0d06 commit 6ae474c7894a6bee222d5b18e59a44fdbf57843a
Showing with 110 additions and 14 deletions.
  1. +84 −0 src/gtests/gtests_searchd.cpp
  2. +6 −6 src/searchd.cpp
  3. +6 −5 src/searchdha.cpp
  4. +14 −3 src/sphinxstd.h
@@ -260,4 +260,88 @@ TEST ( searchd_stuff, prepare_emulation )
PrepareQueryEmulation ( &tQuery );

ASSERT_EQ ( tQuery.m_eRanker, SPH_RANK_PROXIMITY );
}

class CustomNetloop_c : public ::testing::Test
{
protected:
void SetUp () override
{
m_pPoll = sphCreatePoll ( 1000 );
int64_t tmNow = sphMicroTimer ();
SetupEvent ( new CSphWakeupEvent, tmNow );
SetupEvent ( new CSphWakeupEvent, tmNow );
SetupEvent ( new CSphWakeupEvent, tmNow );
}

void TearDown () override
{
m_pPoll->ForAll ( [] ( NetPollEvent_t * pWork ) {
SafeDelete ( pWork );
} );
SafeDelete ( m_pPoll );
}

void SetupEvent ( ISphNetAction * pWork, int64_t tmNow )
{

NetEvent_e eSetup = pWork->Setup ( tmNow );
pWork->m_uNetEvents = ( eSetup==NE_IN ? NetPollEvent_t::READ : NetPollEvent_t::WRITE );
m_pPoll->SetupEvent ( pWork );
}

CSphVector<ISphNetAction *> RemoveOutdated ( int iOutdate, int iOutdate2=-1 )
{
CSphVector<ISphNetAction *> dCleanup;
int ev = -1;
// remove outdated items on no signals
m_pPoll->ForAll ( [&] ( NetPollEvent_t * pEvent ) {
++ev;
if ( ev!=iOutdate && ev!=iOutdate2)
return;

m_pPoll->RemoveEvent ( pEvent );
auto * pWork = (ISphNetAction *) pEvent;
dCleanup.Add ( pWork );
} );
return dCleanup;
}

ISphNetPoller * m_pPoll = nullptr;
};

TEST_F ( CustomNetloop_c, test_usual_remove_1st )
{
int to_del = 0;

auto dOutdated = RemoveOutdated ( to_del );
ARRAY_FOREACH ( i, dOutdated ) SafeDelete ( dOutdated[i] );
}

TEST_F ( CustomNetloop_c, test_usual_remove_2nd )
{
int to_del = 1;

auto dOutdated = RemoveOutdated ( to_del );
ARRAY_FOREACH ( i, dOutdated ) SafeDelete ( dOutdated[i] );
}

TEST_F ( CustomNetloop_c, test_usual_remove_3rd )
{
int to_del = 2;

auto dOutdated = RemoveOutdated ( to_del );
ARRAY_FOREACH ( i, dOutdated ) SafeDelete ( dOutdated[i] );
}

TEST_F ( CustomNetloop_c, test_usual_remove_12 )
{
auto dOutdated = RemoveOutdated ( 0,1 );
ARRAY_FOREACH ( i, dOutdated ) SafeDelete ( dOutdated[i] );
}

TEST_F ( CustomNetloop_c, test_usual_remove_23 )
{
auto dOutdated = RemoveOutdated ( 1, 2 );
ARRAY_FOREACH ( i, dOutdated ) SafeDelete ( dOutdated[i] );
}
// remove outdated items on no signals
m_pPoll->ForAll ([&] ( NetPollEvent_t * pEvent )
{
if ( pEvent->m_iTimeoutTime<=0 || tmNow<pEvent->m_iTimeoutTime )
auto * pWork = (ISphNetAction *) pEvent;
if ( pWork->m_iTimeoutTime<=0 || tmNow<pWork->m_iTimeoutTime )
return;

sphLogDebugv ( "%p bailing on timeout no signal, sock=%d", pEvent, pEvent->m_iSock );
m_pPoll->RemoveEvent ( pEvent );
sphLogDebugv ( "%p bailing on timeout no signal, sock=%d", pWork, pWork->m_iSock );
m_pPoll->RemoveEvent ( pWork );

// close socket immediately to prevent write by client into persist connection that just timed out
// that does not need in case Work got removed at IterateRemove + SafeDelete ( pWork );
// but deferred clean up by ThdJobCleanup_t needs to close socket here right after
// it was removed from e(poll) set
auto * pWork = (ISphNetAction *) pEvent;
pWork->CloseSocket ();
dCleanup.Add ( pWork );
});
&& m_tHeadParser.HeaderFound ( m_tState->m_dDecrypted.Begin(), m_tState->m_dDecrypted.GetLength() )
&& m_tHeadParser.m_iHeaderEnd + m_tHeadParser.m_iFieldContentLenVal>=m_tState->m_dDecrypted.GetLength() )
{
pLoop->RemoveIterEvent ( this );
StartJob ( pLoop );
return NE_REMOVED;
}
&& m_tHeadParser.HeaderFound ( m_tState->m_dDecrypted.Begin(), m_tState->m_dDecrypted.GetLength() )
&& m_tHeadParser.m_iHeaderEnd + m_tHeadParser.m_iFieldContentLenVal>=m_tState->m_dDecrypted.GetLength() )
{
pLoop->RemoveIterEvent ( this );
StartJob ( pLoop );
return NE_REMOVED;
}

void NetReceiveDataHttps_t::StartJob ( CSphNetLoop * pLoop )
{
pLoop->RemoveIterEvent(this);

sphLogDebugv ( "%p HTTPS buf=%d, '%.*s', header=%d, content-len=%d, sock=%d, tick=%u", this, m_tState->m_dDecrypted.GetLength(), Min ( m_tState->m_dDecrypted.GetLength(), 128 ), m_tState->m_dDecrypted.Begin(),
m_tHeadParser.m_iHeaderEnd, m_tHeadParser.m_iFieldContentLenVal, m_iSock, pLoop->m_uTick );

@@ -3885,16 +3885,17 @@ struct EventsList_t

void RemoveEvent ( NetPollEvent_t * pEvent )
{
auto* pList = pEvent->m_tBack.pPtr;
auto* pList = ( NetListedNode_t*) pEvent->m_tBack.pPtr;
m_tWorkList.Remove ( pList );
SafeDelete( pList );
SafeDelete ( pList );
pEvent->m_tBack.pPtr = nullptr;
}

virtual ~EventsList_t ()
{
while ( m_tWorkList.GetLength() )
{
auto * pItem = (ListedData_t *)m_tWorkList.Begin();
auto * pItem = (NetListedNode_t *)m_tWorkList.Begin();
m_tWorkList.Remove ( pItem );
SafeDelete( pItem );
}
@@ -4222,8 +4223,8 @@ class KqueueEvents_c : public ISphNetPoller, public EventsList_t, public Timeout
sphLogDebugv ( "%p kqueue remove, ev=0x%u, sock=%d",
pEvent->m_tBack.pPtr, pEvent->m_uNetEvents, pEvent->m_iSock );
struct kevent tEv[2];
EV_SET( &tEv[0], pEvent->m_iSock, EVFILT_WRITE, EV_DELETE, 0, 0, nullptr );
EV_SET( &tEv[0], pEvent->m_iSock, EVFILT_READ, EV_DELETE, 0, 0, nullptr );
EV_SET( &tEv[0], pEvent->m_iSock, EVFILT_WRITE, EV_DELETE | EV_CLEAR, 0, 0, nullptr );
EV_SET( &tEv[1], pEvent->m_iSock, EVFILT_READ, EV_DELETE | EV_CLEAR, 0, 0, nullptr );
int iRes = kevent ( m_iKQ, tEv, 2, nullptr, 0, nullptr );

// might be already closed by worker from thread pool
@@ -5049,6 +5049,8 @@ class List_t
/// Append the node to the tail
void Add ( ListNode_t * pNode )
{
if ( !pNode )
return;
assert ( !pNode->m_pNext && !pNode->m_pPrev );
pNode->m_pNext = m_tStub.m_pNext;
pNode->m_pPrev = &m_tStub;
@@ -5060,10 +5062,12 @@ class List_t

void Remove ( ListNode_t * pNode )
{
if ( !pNode )
return;
assert ( pNode->m_pNext && pNode->m_pPrev );
pNode->m_pNext->m_pPrev = pNode->m_pPrev;
pNode->m_pPrev->m_pNext = pNode->m_pNext;
//pNode->m_pNext = nullptr; // leave as is for iterate and delete
pNode->m_pNext = nullptr;
pNode->m_pPrev = nullptr;

--m_iCount;
@@ -5087,14 +5091,21 @@ class List_t
class Iterator_c
{
ListNode_t * m_pIterator = nullptr;
ListNode_t * m_pNext = nullptr; // backup since original m.b. corrupted by dtr/free
public:
explicit Iterator_c ( ListNode_t * pIterator = nullptr ) : m_pIterator ( pIterator ) {}
explicit Iterator_c ( ListNode_t * pIterator = nullptr ) : m_pIterator ( pIterator )
{
if ( m_pIterator )
m_pNext = m_pIterator->m_pNext;
}

ListNode_t & operator* () { return *m_pIterator; };

Iterator_c & operator++ ()
{
m_pIterator = m_pIterator->m_pNext;
assert ( m_pNext );
m_pIterator = m_pNext;
m_pNext = m_pIterator->m_pNext;
return *this;
}

0 comments on commit 6ae474c

Please sign in to comment.
You can’t perform that action at this time.