From 6890129bf2aaccbf454dfcf5e8061cdf08993599 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:03:59 +0300 Subject: [PATCH 1/3] Bug #438, http://bugs.skysql.com/show_bug.cgi?id=438 try to complete the fix. dcb->authlock was double-freed. --- server/modules/protocol/mysql_backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index e4b461dad..fff9a2604 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -326,7 +326,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->client->session == NULL) { rc = 1; - goto return_with_lock; + goto return_rc; } usleep(1); } From a73c9c8076e1a5bb9f32047d12480105782aef95 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:36:53 +0300 Subject: [PATCH 2/3] Completing fix to #438 --- server/modules/protocol/mysql_backend.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index fff9a2604..9fe9f72ae 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -282,7 +282,8 @@ static int gw_read_backend_event(DCB *dcb) { } /* switch */ } - if (backend_protocol->state == MYSQL_AUTH_FAILED) { + if (backend_protocol->state == MYSQL_AUTH_FAILED) + { /** * protocol state won't change anymore, * lock can be freed @@ -333,7 +334,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->state == SESSION_STATE_STOPPING) { - goto return_with_lock; + goto return_rc; } spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; From 1a2b8e5475ec254279ce24071f8c3cbd92b65bc8 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 18:29:02 +0300 Subject: [PATCH 3/3] Router spinlock wasn't used to protect routing. As a consequence router could have been closed in thread #1 while thread #2 was in a middle of execution of router code. Solved by holding router lock so that it covered whole routing operation. --- server/modules/protocol/mysql_backend.c | 3 +- .../routing/readwritesplit/readwritesplit.c | 41 +++++++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 9fe9f72ae..efa92a71d 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -585,7 +585,8 @@ gw_MySQLWrite_backend(DCB *dcb, GWBUF *queue) snprintf(str, len+1, "%s", startpoint); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : Authentication to backend failed."))); + "Error : Unable to write to backend due to " + "authentication failure."))); /** Consume query buffer */ while ((queue = gwbuf_consume( queue, diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index a203a9865..5b855afe4 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1015,16 +1015,16 @@ static int routeQuery( router_cli_ses->rses_id))); ss_dassert(QUERY_IS_TYPE(qtype, QUERY_TYPE_READ)); + /** Lock router session */ + if (!rses_begin_locked_router_action(router_cli_ses)) + { + goto return_ret; + } + succp = get_dcb(&slave_dcb, router_cli_ses, BE_SLAVE); if (succp) - { - /** Lock router session */ - if (!rses_begin_locked_router_action(router_cli_ses)) - { - goto return_ret; - } - + { if ((ret = slave_dcb->func.write(slave_dcb, querybuf)) == 1) { atomic_add(&inst->stats.n_slave, 1); @@ -1036,8 +1036,9 @@ static int routeQuery( "Error : Routing query \"%s\" failed.", querystr))); } - rses_end_locked_router_action(router_cli_ses); } + rses_end_locked_router_action(router_cli_ses); + ss_dassert(succp); goto return_ret; } @@ -1061,6 +1062,11 @@ static int routeQuery( "routing to Master."))); } } + /** Lock router session */ + if (!rses_begin_locked_router_action(router_cli_ses)) + { + goto return_ret; + } if (master_dcb == NULL) { @@ -1068,21 +1074,22 @@ static int routeQuery( } if (succp) { - /** Lock router session */ - if (!rses_begin_locked_router_action(router_cli_ses)) - { - goto return_ret; - } if ((ret = master_dcb->func.write(master_dcb, querybuf)) == 1) { atomic_add(&inst->stats.n_master, 1); } - rses_end_locked_router_action(router_cli_ses); - } + } + rses_end_locked_router_action(router_cli_ses); + ss_dassert(succp); - ss_dassert(ret == 1); - goto return_ret; + + if (ret == 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Routing to master failed."))); + } } return_ret: if (plainsqlbuf != NULL)