Re: bzr commit into mysql-trunk-bugfixing branch (Georgi.Kodinov:3435) Bug#52515

[Nirbhay Choubey]

Hi Joro,

I have noticed a change after applying this patch (on windows):

Execute,  select sleep(10000);
Wait for a few seconds & Press 'CTRL C'
This will cause client to exit too.

mysql> select sleep(10000);
terminal close -- sending "KILL QUERY 44" to server ...
terminal close -- query aborted.
^C
 >

Notice the 'cause' above, it shows 'terminal close', but should have been,
'Ctrl-C'.

Besides this, a suggestion on code change below (inline) :

On 12/16/2010 6:23 PM, Georgi Kodinov wrote:
> #At file:///D:/mysql/work/B52515-trunk-bugfixing/ based on
> revid:jon.hauglid@stripped
>
>   3435 Georgi Kodinov	2010-12-16
>        Bug #52515: mysql sessions are not terminated properly
>
>        The mysql command line client was not reacting at all on
>        closing its controlling terminal.
>        Added windows specific code to handle the console events.
>        Added a handler for the SIGHUP to gracefully shutdown
>        the running statement.
>
>      modified:
>        client/mysql.cc
> === modified file 'client/mysql.cc'
> --- a/client/mysql.cc	2010-11-26 14:37:59 +0000
> +++ b/client/mysql.cc	2010-12-16 12:53:50 +0000
> @@ -1059,12 +1059,33 @@ static void end_timer(ulong start_time,c
>   static void mysql_end_timer(ulong start_time,char *buff);
>   static void nice_time(double sec,char *buff,bool part_second);
>   extern "C" sig_handler mysql_end(int sig);
> -extern "C" sig_handler handle_sigint(int sig);
> +extern "C" sig_handler handle_kill_signal(int sig);
>   #if defined(HAVE_TERMIOS_H)&&  defined(GWINSZ_IN_SYS_IOCTL)
>   static sig_handler window_resize(int sig);
>   #endif
>
>
> +
> +#ifdef _WIN32
> +BOOL win32_ctrl_handler(DWORD fdwCtrlType)
> +{
> +  switch (fdwCtrlType)
> +  {
> +  case CTRL_C_EVENT:
> +    if (opt_sigint_ignore)
> +      return FALSE;
> +  case CTRL_BREAK_EVENT:
> +  case CTRL_CLOSE_EVENT:
> +  case CTRL_LOGOFF_EVENT:
> +  case CTRL_SHUTDOWN_EVENT:
> +    handle_kill_signal(SIGINT + 1);
> +  }
> +  return FALSE;
> +
> +}
> +#endif
> +
> +
>   int main(int argc,char *argv[])
>   {
>     char buff[80];
> @@ -1154,11 +1175,17 @@ int main(int argc,char *argv[])
>     if (!status.batch)
>       ignore_errors=1;				// Don't abort monitor
>
> +#ifndef _WIN32
>     if (opt_sigint_ignore)
>       signal(SIGINT, SIG_IGN);
>     else
> -    signal(SIGINT, handle_sigint);              // Catch SIGINT to clean up
> +    signal(SIGINT, handle_kill_signal);         // Catch SIGINT to clean up
>     signal(SIGQUIT, mysql_end);			// Catch SIGQUIT to clean up
> +  signal(SIGHUP, handle_kill_signal);         // Catch SIGHUP to clean up
> +#else
> +  SetConsoleCtrlHandler((PHANDLER_ROUTINE) windows_ctrl_handler, TRUE);

You might want to rename 'windows_ctrl_handler' to 'win32_ctrl_handler' 
or otherwise.

This causes build failure on Windows.

BTW, I am not able to reproduce this bug on windows even with the pre-patch
code.

Best Regards,
Nirbhay

> +#endif
> +
>
>   #if defined(HAVE_TERMIOS_H)&&  defined(GWINSZ_IN_SYS_IOCTL)
>     /* Readline will call this if it installs a handler */
> @@ -1285,16 +1312,17 @@ sig_handler mysql_end(int sig)
>     If query is in process, kill query
>     no query in process, terminate like previous behavior
>    */
> -sig_handler handle_sigint(int sig)
> +sig_handler handle_kill_signal(int sig)
>   {
>     char kill_buffer[40];
>     MYSQL *kill_mysql= NULL;
> +  const char *reason = sig == SIGINT ? "Ctrl-C" : "terminal close";
>
>     /* terminate if no query being executed, or we already tried interrupting */
>     /* terminate if no query being executed, or we already tried interrupting */
>     if (!executing_query || (interrupted_query == 2))
>     {
> -    tee_fprintf(stdout, "Ctrl-C -- exit!\n");
> +    tee_fprintf(stdout, "%s -- exit!\n", reason);
>       goto err;
>     }
>
> @@ -1302,7 +1330,7 @@ sig_handler handle_sigint(int sig)
>     if (!mysql_real_connect(kill_mysql,current_host, current_user, opt_password,
>                             "", opt_mysql_port, opt_mysql_unix_port,0))
>     {
> -    tee_fprintf(stdout, "Ctrl-C -- sorry, cannot connect to server to kill query,
> giving up ...\n");
> +    tee_fprintf(stdout, "%s -- sorry, cannot connect to server to kill query, giving
> up ...\n", reason);
>       goto err;
>     }
>
> @@ -1316,10 +1344,11 @@ sig_handler handle_sigint(int sig)
>     sprintf(kill_buffer, "KILL %s%lu",
>             (interrupted_query == 1) ? "QUERY " : "",
>             mysql_thread_id(&mysql));
> -  tee_fprintf(stdout, "Ctrl-C -- sending \"%s\" to server ...\n", kill_buffer);
> +  tee_fprintf(stdout, "%s -- sending \"%s\" to server ...\n",
> +              reason, kill_buffer);
>     mysql_real_query(kill_mysql, kill_buffer, (uint) strlen(kill_buffer));
>     mysql_close(kill_mysql);
> -  tee_fprintf(stdout, "Ctrl-C -- query aborted.\n");
> +  tee_fprintf(stdout, "%s -- query aborted.\n", reason);
>
>     return;
>
>
>
>
>