Various changes to block loopholes in different cases and tidy up.

This commit is contained in:
counterpoint
2015-09-21 09:23:22 +01:00
parent b8af047a25
commit 88716c35fb
5 changed files with 122 additions and 63 deletions

View File

@ -289,10 +289,10 @@ dcb_final_free(DCB *dcb)
{ {
DCB_CALLBACK *cb; DCB_CALLBACK *cb;
CHK_DCB(dcb); CHK_DCB(dcb);
ss_info_dassert(dcb->state == DCB_STATE_DISCONNECTED || ss_info_dassert(dcb->state == DCB_STATE_DISCONNECTED ||
dcb->state == DCB_STATE_ALLOC, dcb->state == DCB_STATE_ALLOC,
"dcb not in DCB_STATE_DISCONNECTED not in DCB_STATE_ALLOC state."); "dcb not in DCB_STATE_DISCONNECTED not in DCB_STATE_ALLOC state.");
if (DCB_POLL_BUSY(dcb)) if (DCB_POLL_BUSY(dcb))
{ {
@ -326,53 +326,58 @@ dcb_final_free(DCB *dcb)
if (ptr) if (ptr)
ptr->next = dcb->next; ptr->next = dcb->next;
} }
spinlock_release(&dcbspin); spinlock_release(&dcbspin);
if (dcb->session) { if (dcb->session) {
/*< /*<
* Terminate client session. * Terminate client session.
*/ */
{ SESSION *local_session = dcb->session;
SESSION *local_session = dcb->session; dcb->session = NULL;
dcb->session = NULL; CHK_SESSION(local_session);
CHK_SESSION(local_session); /**
/** * Set session's client pointer NULL so that other threads
* Set session's client pointer NULL so that other threads * won't try to call dcb_close for client DCB
* won't try to call dcb_close for client DCB * after this call.
* after this call. */
*/ if (local_session->client == dcb)
if (local_session->client == dcb) {
{ spinlock_acquire(&local_session->ses_lock);
spinlock_acquire(&local_session->ses_lock); local_session->client = NULL;
local_session->client = NULL; spinlock_release(&local_session->ses_lock);
spinlock_release(&local_session->ses_lock); }
} if (SESSION_STATE_DUMMY != local_session->state)
if (SESSION_STATE_DUMMY != local_session->state) {
{ session_free(local_session);
session_free(local_session); }
}
}
} }
if (dcb->protocol && (!DCB_IS_CLONE(dcb))) if (dcb->protocol && (!DCB_IS_CLONE(dcb)))
free(dcb->protocol); free(dcb->protocol);
if (dcb->protoname) if (dcb->protoname)
free(dcb->protoname); free(dcb->protoname);
if (dcb->remote) if (dcb->remote)
free(dcb->remote); free(dcb->remote);
if (dcb->user) if (dcb->user)
free(dcb->user); free(dcb->user);
/* Clear write and read buffers */ /* Clear write and read buffers */
if (dcb->delayq) { if (dcb->delayq) {
GWBUF *queue = dcb->delayq; GWBUF *queue = dcb->delayq;
while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL);
} dcb->delayq = NULL;
if (dcb->dcb_readqueue) }
{ if (dcb->writeq) {
GWBUF* queue = dcb->dcb_readqueue; GWBUF *queue = dcb->writeq;
while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL);
} dcb->writeq = NULL;
}
if (dcb->dcb_readqueue)
{
GWBUF* queue = dcb->dcb_readqueue;
while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL);
dcb->dcb_readqueue = NULL;
}
spinlock_acquire(&dcb->cb_lock); spinlock_acquire(&dcb->cb_lock);
while ((cb = dcb->callbacks) != NULL) while ((cb = dcb->callbacks) != NULL)
@ -549,10 +554,36 @@ dcb_process_victim_queue(DCB *listofdcb)
dcb = dcb->memdata.next; dcb = dcb->memdata.next;
continue; continue;
} }
else
{
DCB *nextdcb;
poll_remove_dcb(dcb);
spinlock_acquire(&zombiespin);
bitmask_copy(&dcb->memdata.bitmask, poll_bitmask());
nextdcb = dcb->memdata.next;
dcb->memdata.next = zombies;
zombies = dcb;
spinlock_release(&zombiespin);
if (dcb->server)
{
atomic_add(&dcb->server->stats.n_current, -1);
}
dcb = nextdcb;
continue;
}
} }
dcb_close_finish(dcb);
} }
/**
* close protocol and router session
*/
if (dcb->func.close != NULL)
{
dcb->func.close(dcb);
}
/** Call possible callback for this DCB in case of close */
dcb_call_callback(dcb, DCB_REASON_CLOSE);
if (dcb->fd > 0) if (dcb->fd > 0)
{ {
/*< /*<
@ -1826,7 +1857,7 @@ dcb_close(DCB *dcb)
pthread_self(), pthread_self(),
dcb, dcb,
STRDCBSTATE(dcb->state)))); STRDCBSTATE(dcb->state))));
raise(SIGABRT); raise(SIGABRT);
} }
/** /**
@ -1902,6 +1933,20 @@ dcb_maybe_add_persistent(DCB *dcb)
dcb->user))); dcb->user)));
dcb->dcb_is_zombie = false; dcb->dcb_is_zombie = false;
dcb->persistentstart = time(NULL); dcb->persistentstart = time(NULL);
if (dcb->session)
/*<
* Terminate client session.
*/
{
SESSION *local_session = dcb->session;
session_set_dummy(dcb);
CHK_SESSION(local_session);
if (SESSION_STATE_DUMMY != local_session->state)
{
session_free(local_session);
}
}
dcb->callbacks = NULL;
spinlock_acquire(&dcb->server->persistlock); spinlock_acquire(&dcb->server->persistlock);
dcb->nextpersistent = dcb->server->persistent; dcb->nextpersistent = dcb->server->persistent;
dcb->server->persistent = dcb; dcb->server->persistent = dcb;
@ -2063,6 +2108,15 @@ dprintOneDCB(DCB *pdcb, DCB *dcb)
if (dcb->remote) if (dcb->remote)
dcb_printf(pdcb, "\tConnected to: %s\n", dcb_printf(pdcb, "\tConnected to: %s\n",
dcb->remote); dcb->remote);
if (dcb->server)
{
if (dcb->server->name)
dcb_printf(pdcb, "\tServer name/IP: %s\n",
dcb->server->name);
if (dcb->server->port)
dcb_printf(pdcb, "\tPort number: %d\n",
dcb->server->port);
}
if (dcb->user) if (dcb->user)
dcb_printf(pdcb, "\tUsername: %s\n", dcb_printf(pdcb, "\tUsername: %s\n",
dcb->user); dcb->user);
@ -2858,7 +2912,10 @@ dcb_persistent_clean_count(DCB *dcb, bool cleanall)
{ {
nextdcb = disposals->nextpersistent; nextdcb = disposals->nextpersistent;
disposals->persistentstart = 0; disposals->persistentstart = 0;
dcb_close_finish(disposals); if (DCB_STATE_POLLING == dcb->state)
{
poll_remove_dcb(dcb);
}
dcb_close(disposals); dcb_close(disposals);
disposals = nextdcb; disposals = nextdcb;
} }

View File

@ -860,7 +860,12 @@ unsigned long qtime;
#endif /* FAKE_CODE */ #endif /* FAKE_CODE */
ss_debug(spinlock_acquire(&dcb->dcb_initlock);) ss_debug(spinlock_acquire(&dcb->dcb_initlock);)
ss_dassert(dcb->state != DCB_STATE_ALLOC); ss_dassert(dcb->state != DCB_STATE_ALLOC);
ss_dassert(dcb->state != DCB_STATE_DISCONNECTED); /* It isn't obvious that this is impossible */
/* ss_dassert(dcb->state != DCB_STATE_DISCONNECTED); */
if (DCB_STATE_DISCONNECTED == dcb->state)
{
return 0;
}
ss_debug(spinlock_release(&dcb->dcb_initlock);) ss_debug(spinlock_release(&dcb->dcb_initlock);)
LOGIF(LD, (skygw_log_write( LOGIF(LD, (skygw_log_write(

View File

@ -167,7 +167,7 @@ static int gw_read_backend_event(DCB *dcb) {
int rc = 0; int rc = 0;
CHK_DCB(dcb); CHK_DCB(dcb);
if (!dcb->session && dcb->persistentstart) if (dcb->persistentstart)
{ {
dcb->dcb_errhandle_called = true; dcb->dcb_errhandle_called = true;
goto return_rc; goto return_rc;
@ -407,16 +407,11 @@ static int gw_read_backend_event(DCB *dcb) {
else else
{ {
dcb->dcb_errhandle_called = true; dcb->dcb_errhandle_called = true;
dcb_close(dcb);
rc = 1;
goto return_rc;
} }
gwbuf_free(errbuf); gwbuf_free(errbuf);
LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG,
"%lu [gw_read_backend_event] "
"after calling handleError. Backend "
"DCB %p, session %p",
pthread_self(),
dcb,
dcb->session)));
spinlock_acquire(&session->ses_lock); spinlock_acquire(&session->ses_lock);
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
@ -1066,7 +1061,7 @@ gw_backend_hangup(DCB *dcb)
session_state_t ses_state; session_state_t ses_state;
CHK_DCB(dcb); CHK_DCB(dcb);
if (!dcb->session && dcb->persistentstart) if (dcb->persistentstart)
{ {
dcb->dcb_errhandle_called = true; dcb->dcb_errhandle_called = true;
goto retblock; goto retblock;
@ -1124,6 +1119,7 @@ gw_backend_hangup(DCB *dcb)
} }
} }
gwbuf_free(errbuf); gwbuf_free(errbuf);
dcb_close(dcb);
goto retblock; goto retblock;
} }
#if defined(SS_DEBUG) #if defined(SS_DEBUG)

View File

@ -886,7 +886,7 @@ static void handleError(
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
} }
if (router_cli_ses->backend_dcb) { if (router_cli_ses && router_cli_ses->backend_dcb) {
if (backend_dcb != router_cli_ses->backend_dcb) if (backend_dcb != router_cli_ses->backend_dcb)
{ {
/* Linkages have gone badly wrong */ /* Linkages have gone badly wrong */
@ -898,9 +898,9 @@ static void handleError(
backend_dcb))); backend_dcb)));
} }
router_cli_ses->backend_dcb = NULL; router_cli_ses->backend_dcb = NULL;
dcb_close(backend_dcb);
} }
dcb_close(backend_dcb);
/** false because connection is not available anymore */ /** false because connection is not available anymore */
*succp = false; *succp = false;
} }

View File

@ -990,7 +990,7 @@ static void closeSession(
int i; int i;
/** /**
* This sets router closed. Nobody is allowed to use router * This sets router closed. Nobody is allowed to use router
* whithout checking this first. * without checking this first.
*/ */
router_cli_ses->rses_closed = true; router_cli_ses->rses_closed = true;
@ -5306,8 +5306,6 @@ static int router_handle_state_switch(
backend_ref_t* bref; backend_ref_t* bref;
int rc = 1; int rc = 1;
SERVER* srv; SERVER* srv;
ROUTER_CLIENT_SES* rses;
SESSION* ses;
CHK_DCB(dcb); CHK_DCB(dcb);
bref = (backend_ref_t *)data; bref = (backend_ref_t *)data;
CHK_BACKEND_REF(bref); CHK_BACKEND_REF(bref);
@ -5327,7 +5325,10 @@ static int router_handle_state_switch(
srv->port, srv->port,
STRSRVSTATUS(srv)))); STRSRVSTATUS(srv))));
CHK_SESSION(((SESSION*)dcb->session)); CHK_SESSION(((SESSION*)dcb->session));
CHK_CLIENT_RSES(((ROUTER_CLIENT_SES *)dcb->session->router_session)); if (dcb->session->router_session)
{
CHK_CLIENT_RSES(((ROUTER_CLIENT_SES *)dcb->session->router_session));
}
switch (reason) { switch (reason) {
case DCB_REASON_NOT_RESPONDING: case DCB_REASON_NOT_RESPONDING: