Details

    Description

      Control-c should not exit the client. It should cancel partial input or abort the current statement if there is one, but it should not do anything else otherwise.
      Control-d should exit the client if the input line is empty.

      This is the expected behavior for interactive applications. The current behavior is unexpected and disruptive.

      (Yes, I know about --sigint-ignore, but that is a poor solution.)

      This bug was fixed in mysql 5.7: https://bugs.mysql.com/bug.php?id=66583

      Attachments

        1. screenshot-1.png
          screenshot-1.png
          41 kB
        2. screenshot-2.png
          screenshot-2.png
          76 kB
        3. screenshot-3.png
          screenshot-3.png
          33 kB

        Activity

          nkovacs Nikola Kovacs created issue -
          elenst Elena Stepanova made changes -
          Field Original Value New Value
          Fix Version/s 10.3 [ 22126 ]
          Affects Version/s 5.5 [ 15800 ]
          Affects Version/s 10.0 [ 16000 ]
          Affects Version/s 10.1 [ 16100 ]
          Affects Version/s 10.2 [ 14601 ]
          Affects Version/s 10.3.2 [ 22533 ]
          Labels upstream-fixed
          elenst Elena Stepanova made changes -
          Status Open [ 1 ] Confirmed [ 10101 ]
          elenst Elena Stepanova made changes -
          nkovacs Nikola Kovacs made changes -
          Description Control-c should not exit the client. It should cancel partial input or abort the current statement if there is none, but it should not do anything else otherwise.
          Control-d should exit the client if the input line is empty.

          This is the expected behavior for interactive applications. The current behavior is unexpected and disruptive.

          (Yes, I know about --sigint-ignore, but that is a poor solution.)

          This bug was fixed in mysql 5.7: https://bugs.mysql.com/bug.php?id=66583
          Control-c should not exit the client. It should cancel partial input or abort the current statement if there is one, but it should not do anything else otherwise.
          Control-d should exit the client if the input line is empty.

          This is the expected behavior for interactive applications. The current behavior is unexpected and disruptive.

          (Yes, I know about --sigint-ignore, but that is a poor solution.)

          This bug was fixed in mysql 5.7: https://bugs.mysql.com/bug.php?id=66583
          serg Sergei Golubchik made changes -
          Labels upstream-fixed beginner-friendly upstream-fixed
          serg Sergei Golubchik made changes -
          Fix Version/s 10.3 [ 22126 ]
          serg Sergei Golubchik made changes -
          Fix Version/s 10.2 [ 14601 ]
          Fix Version/s 10.3 [ 22126 ]
          Fix Version/s 5.5 [ 15800 ]
          Fix Version/s 10.0 [ 16000 ]
          Fix Version/s 10.1 [ 16100 ]
          svoj Sergey Vojtovich made changes -
          Labels beginner-friendly upstream-fixed beginner-friendly contribution foundation upstream-fixed
          svoj Sergey Vojtovich made changes -
          Assignee Vicentiu Ciorbaru [ cvicentiu ]
          serg Sergei Golubchik made changes -
          Fix Version/s 10.4 [ 22408 ]
          anel Anel Husakovic made changes -
          Assignee Vicentiu Ciorbaru [ cvicentiu ] Anel Husakovic [ anel ]
          anel Anel Husakovic made changes -
          Status Confirmed [ 10101 ] In Review [ 10002 ]
          anel Anel Husakovic made changes -
          Status In Review [ 10002 ] Stalled [ 10000 ]
          anel Anel Husakovic made changes -
          Comment [ PR#703 closed since it is copy of mysql which is under GPLv2 ]
          anel Anel Husakovic made changes -
          Status Stalled [ 10000 ] In Progress [ 3 ]
          anel Anel Husakovic made changes -
          Status In Progress [ 3 ] Stalled [ 10000 ]
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Sergei Golubchik [ serg ]
          Status Stalled [ 10000 ] In Review [ 10002 ]
          serg Sergei Golubchik made changes -
          Assignee Sergei Golubchik [ serg ] Anel Husakovic [ anel ]
          Status In Review [ 10002 ] Stalled [ 10000 ]

          Fixed version with patch on 10.2, if needed I can switch to other one. Specification only require linux env.

          anel Anel Husakovic added a comment - Fixed version with patch on 10.2, if needed I can switch to other one. Specification only require linux env.
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Sergei Golubchik [ serg ]
          Status Stalled [ 10000 ] In Review [ 10002 ]
          serg Sergei Golubchik made changes -
          Assignee Sergei Golubchik [ serg ] Anel Husakovic [ anel ]
          Status In Review [ 10002 ] Stalled [ 10000 ]
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Sergei Golubchik [ serg ]
          Status Stalled [ 10000 ] In Review [ 10002 ]

          1ba0c5b4c49688d4ad9548daed52c9ca57cd002b is ok to push

          serg Sergei Golubchik added a comment - 1ba0c5b4c49688d4ad9548daed52c9ca57cd002b is ok to push
          serg Sergei Golubchik made changes -
          Assignee Sergei Golubchik [ serg ] Anel Husakovic [ anel ]
          Status In Review [ 10002 ] Stalled [ 10000 ]

          Pushed with 396313d301b3567aea.

          anel Anel Husakovic added a comment - Pushed with 396313d301b3567aea .
          anel Anel Husakovic made changes -
          Component/s Scripts & Clients [ 11002 ]
          Fix Version/s 5.5.66 [ 23403 ]
          Fix Version/s 10.1.42 [ 23407 ]
          Fix Version/s 10.2.28 [ 23910 ]
          Fix Version/s 10.3.19 [ 23908 ]
          Fix Version/s 10.4.9 [ 23906 ]
          Fix Version/s 10.2 [ 14601 ]
          Fix Version/s 5.5 [ 15800 ]
          Fix Version/s 10.0 [ 16000 ]
          Fix Version/s 10.1 [ 16100 ]
          Fix Version/s 10.3 [ 22126 ]
          Fix Version/s 10.4 [ 22408 ]
          Resolution Fixed [ 1 ]
          Status Stalled [ 10000 ] Closed [ 6 ]

          Broke Windows build on 5.5. Please test in own tree, before pushing into mainline.

          wlad Vladislav Vaintroub added a comment - Broke Windows build on 5.5. Please test in own tree, before pushing into mainline.

          Reverted the patch.

          It doesn't compile on Windows and it doesn't compile with libedit (e.g. on p8-suse12 in buildbot).

          #ifdef is a wrong fix, it makes the code to compile, but the Ctrl-C behavior is still wrong.

          serg Sergei Golubchik added a comment - Reverted the patch. It doesn't compile on Windows and it doesn't compile with libedit (e.g. on p8-suse12 in buildbot). #ifdef is a wrong fix, it makes the code to compile, but the Ctrl-C behavior is still wrong.
          serg Sergei Golubchik made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Stalled [ 10000 ]
          serg Sergei Golubchik made changes -
          Fix Version/s 5.5 [ 15800 ]
          Fix Version/s 10.1 [ 16100 ]
          Fix Version/s 10.2 [ 14601 ]
          Fix Version/s 10.3 [ 22126 ]
          Fix Version/s 10.4 [ 22408 ]
          Fix Version/s 5.5.66 [ 23403 ]
          Fix Version/s 10.1.42 [ 23407 ]
          Fix Version/s 10.4.9 [ 23906 ]
          Fix Version/s 10.3.19 [ 23908 ]
          Fix Version/s 10.2.28 [ 23910 ]
          julien.fritsch Julien Fritsch made changes -
          Fix Version/s 5.5 [ 15800 ]
          julien.fritsch Julien Fritsch made changes -
          Fix Version/s 10.1 [ 16100 ]
          serg Sergei Golubchik made changes -
          Workflow MariaDB v3 [ 83900 ] MariaDB v4 [ 143522 ]
          anel Anel Husakovic made changes -
          Attachment screenshot-1.png [ 61906 ]
          anel Anel Husakovic made changes -
          Attachment screenshot-2.png [ 61907 ]

          Example of the patch 84c9cbc36b94bcf3
          Windows:

          Ubuntu:

          anel Anel Husakovic added a comment - Example of the patch 84c9cbc36b94bcf3 Windows: Ubuntu:
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Sergei Golubchik [ serg ]
          Status Stalled [ 10000 ] In Review [ 10002 ]
          nkovacs Nikola Kovacs added a comment -

          @anel won't this still terminate the client if you mash ctrl-c while a query is executing (if (interrupted_query == 2))?

          nkovacs Nikola Kovacs added a comment - @anel won't this still terminate the client if you mash ctrl-c while a query is executing (if (interrupted_query == 2))?

          Hi, I think it won't since interrupted_query == 2 according to the code means that with first interrupt the query will be killed and after that connection and this happens only for server version < 5, which doesn't recognize kill query part.

          anel Anel Husakovic added a comment - Hi, I think it won't since interrupted_query == 2 according to the code means that with first interrupt the query will be killed and after that connection and this happens only for server version < 5, which doesn't recognize kill query part.
          nkovacs Nikola Kovacs added a comment - - edited

          If the sigint handler is invoked three times (two times with server < 5) before com_go resets interrupted_query to 0, it will terminate the client. This can happen if you have a slow connection and press ctrl-c multiple times.

          How to reproduce:

          1. start a mariadb docker image
          2. add a 1s delay to your docker network using tc: `tc qdisc add dev docker0 root netem delay 1000ms`
          3. run `show databases;` and immediately press ctrl-c twice once works too
          4. after "Ctrl-C – query killed. Continuing normally." appears, quickly press ctrl-c again.

          Edit: alternatively:

          1. run `show databases;` and immediately press ctrl-c twice
          2. You'll get "Cltr-C – query killed. Continuing normally" twice, but you'll also get the query result and the prompt back
          3. Now press cltr-c on the empty prompt and it will exit
          nkovacs Nikola Kovacs added a comment - - edited If the sigint handler is invoked three times (two times with server < 5) before com_go resets interrupted_query to 0, it will terminate the client. This can happen if you have a slow connection and press ctrl-c multiple times. How to reproduce: start a mariadb docker image add a 1s delay to your docker network using tc: `tc qdisc add dev docker0 root netem delay 1000ms` run `show databases;` and immediately press ctrl-c twice once works too after "Ctrl-C – query killed. Continuing normally." appears, quickly press ctrl-c again. Edit: alternatively: run `show databases;` and immediately press ctrl-c twice You'll get "Cltr-C – query killed. Continuing normally" twice, but you'll also get the query result and the prompt back Now press cltr-c on the empty prompt and it will exit

          Also, I vouch for not changing behavior on Windows. That least astonishment is Unix thing, the bash user's delight.
          On Windows, either actually handle Ctrl-D (or better, Ctrl-Z which would be cultural equivalent), or leave it as-is

          wlad Vladislav Vaintroub added a comment - Also, I vouch for not changing behavior on Windows. That least astonishment is Unix thing, the bash user's delight. On Windows, either actually handle Ctrl-D (or better, Ctrl-Z which would be cultural equivalent), or leave it as-is
          nkovacs Nikola Kovacs added a comment -

          Accidentally terminating the client when you just wanted to cancel a long running query is bad regardless of operating system. But yes, Ctrl-Z should be handled on Windows. The mysql patch for this bug also fixed that.

          nkovacs Nikola Kovacs added a comment - Accidentally terminating the client when you just wanted to cancel a long running query is bad regardless of operating system. But yes, Ctrl-Z should be handled on Windows. The mysql patch for this bug also fixed that.
          wlad Vladislav Vaintroub added a comment - - edited

          I do not think anyone using Windows has complained yet.
          Being the only one to jump in to comment, here is my point - I came to appreciate not having to type "quit", when I want to quit. I prefer it short . Like Ctrl-C. Any other Ctrl-whatever is in principle good to me, except Ctrl-Break - good in theory , but not existent on my keyboard

          Nirbhay's patch you quoted, it does not do exactly what it promises. It does not break on Ctrl-Z on Windows. It does instead break on "Ctrl-Z-<optionally any sequence of characters>-Enter". That "Enter" is one character too much to type, and it also reads the full line, no matter how many character are there, and it does not break with Ctrl-Z the middle of the line, and all of it is the violation of principle of least astonishment, in my understanding. It is incorrect, although easy to implement this way.

          The more complicated, and correct way to do that is different, and it does not involve signal handlers and what not. https://www.dpldocs.info/this-week-in-d/Blog.Posted_2019_11_25.html outlines how to implement that correctly (although it is not C, it is easy to see what is he doing).
          Perhaps we should not expect Anel to make both Unix and Windows users happy in a single patch.

          In fact, the Windows part, I would probably like to patch myself, but first maybe we should make more vocal and angry Linux users happy, while
          not making Windows users like myself unhappy at the same time.

          Lastly, not being raised in strict Unix tradition, I do not find that command line client should necessarily resemble all of the bash, and I can live with Ctrl-C ending the client in principle, and do not get all the buzz around it. Ok, one typed Ctrl-C one time too many, the client died, so what? "ARROW UP"+Enter to reconnect the client, from bash. "ARROW UP"+Enter to repeat the last command in client's history. What is all the buzz about?

          wlad Vladislav Vaintroub added a comment - - edited I do not think anyone using Windows has complained yet. Being the only one to jump in to comment, here is my point - I came to appreciate not having to type "quit", when I want to quit. I prefer it short . Like Ctrl-C. Any other Ctrl-whatever is in principle good to me, except Ctrl-Break - good in theory , but not existent on my keyboard Nirbhay's patch you quoted, it does not do exactly what it promises. It does not break on Ctrl-Z on Windows. It does instead break on "Ctrl-Z-<optionally any sequence of characters>-Enter". That "Enter" is one character too much to type, and it also reads the full line, no matter how many character are there, and it does not break with Ctrl-Z the middle of the line, and all of it is the violation of principle of least astonishment, in my understanding. It is incorrect, although easy to implement this way. The more complicated, and correct way to do that is different, and it does not involve signal handlers and what not. https://www.dpldocs.info/this-week-in-d/Blog.Posted_2019_11_25.html outlines how to implement that correctly (although it is not C, it is easy to see what is he doing). Perhaps we should not expect Anel to make both Unix and Windows users happy in a single patch. In fact, the Windows part, I would probably like to patch myself, but first maybe we should make more vocal and angry Linux users happy, while not making Windows users like myself unhappy at the same time. Lastly, not being raised in strict Unix tradition, I do not find that command line client should necessarily resemble all of the bash, and I can live with Ctrl-C ending the client in principle, and do not get all the buzz around it. Ok, one typed Ctrl-C one time too many, the client died, so what? "ARROW UP"+Enter to reconnect the client, from bash. "ARROW UP"+Enter to repeat the last command in client's history. What is all the buzz about?

          Hi nkovacs, thanks for the example.
          I followed but couldn't obtain the result, can you please check what is wrong ?

          # Start mariadb Docker image
          $ docker ps 
          CONTAINER ID   IMAGE            COMMAND                  CREATED         STATUS         PORTS      NAMES
          bea59acdb00c   mariadb:latest   "docker-entrypoint.s…"   2 minutes ago   Up 2 minutes   3306/tcp   mariadb-server
           
          # Created rule for tc -> tested first to start the container after to create the rule and to remove the container on already existing rule - not working still.
          # $ sudo tc qdisc add dev docker0 root netem delay 1000ms
           
          # Show the rule
           $ tc qdisc show|grep docker0
          qdisc netem 8001: dev docker0 root refcnt 2 limit 1000 delay 1.0s
           
          # 1) Try from within container -> couldn't get latency and preform quick ^C
          $ docker exec -it mariadb-server bash
          MariaDB [(none)]> show databases;
          +--------------------+
          | Database           |
          +--------------------+
          | information_schema |
          | mysql              |
          | performance_schema |
          | sys                |
          +--------------------+
          4 rows in set (0.000 sec)
           
          # 2) Try with exec -> again couldn't get latency and preform quick ^C
          $ docker exec -it mariadb-server mariadb -uroot -psecret
          MariaDB [(none)]> show databases;
          +--------------------+
          | Database           |
          +--------------------+
          | information_schema |
          | mysql              |
          | performance_schema |
          | sys                |
          +--------------------+
          4 rows in set (0.000 sec)
          

          wlad thanks for the reference without sighandler.
          The patch is for ^C only because it failed last time for Windows (I think it works now for Windows), of course we can remove Win patch, but also I can take a look for ^Z - without sighandler, if we decide to go with that and someone confirms or to do that in other MDEV, what may be atm better solution .

          anel Anel Husakovic added a comment - Hi nkovacs , thanks for the example. I followed but couldn't obtain the result, can you please check what is wrong ? # Start mariadb Docker image $ docker ps CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES bea59acdb00c mariadb:latest "docker-entrypoint.s…" 2 minutes ago Up 2 minutes 3306 /tcp mariadb-server   # Created rule for tc -> tested first to start the container after to create the rule and to remove the container on already existing rule - not working still. # $ sudo tc qdisc add dev docker0 root netem delay 1000ms   # Show the rule $ tc qdisc show| grep docker0 qdisc netem 8001: dev docker0 root refcnt 2 limit 1000 delay 1.0s   # 1) Try from within container -> couldn't get latency and preform quick ^C $ docker exec -it mariadb-server bash MariaDB [(none)]> show databases; +--------------------+ | Database | +--------------------+ | information_schema | | mysql | | performance_schema | | sys | +--------------------+ 4 rows in set (0.000 sec)   # 2) Try with exec -> again couldn't get latency and preform quick ^C $ docker exec -it mariadb-server mariadb -uroot -psecret MariaDB [(none)]> show databases; +--------------------+ | Database | +--------------------+ | information_schema | | mysql | | performance_schema | | sys | +--------------------+ 4 rows in set (0.000 sec) wlad thanks for the reference without sighandler. The patch is for ^C only because it failed last time for Windows (I think it works now for Windows), of course we can remove Win patch, but also I can take a look for ^Z - without sighandler, if we decide to go with that and someone confirms or to do that in other MDEV, what may be atm better solution .
          nkovacs Nikola Kovacs added a comment -

          I ran the mariadb client from outside the docker container. I think that may be the issue.

          nkovacs Nikola Kovacs added a comment - I ran the mariadb client from outside the docker container. I think that may be the issue.

          Like this ?

          # 2) Try with exec -> again couldn't get latency and preform quick ^C
          $ docker exec -it mariadb-server mariadb -uroot -psecret
          

          anel Anel Husakovic added a comment - Like this ? # 2) Try with exec -> again couldn't get latency and preform quick ^C $ docker exec -it mariadb-server mariadb -uroot -psecret
          nkovacs Nikola Kovacs added a comment - - edited

          No. I started the mariadb image with -p=3306:3306, then ran the client on my machine, not docker, with -hlocalhost -P3306 --protocol=tcp

          nkovacs Nikola Kovacs added a comment - - edited No. I started the mariadb image with -p=3306:3306, then ran the client on my machine, not docker, with -hlocalhost -P3306 --protocol=tcp
          anel Anel Husakovic added a comment - - edited

          Thanks, but still the same:

          $ tc qdisc show|grep docker
          qdisc noqueue 0: dev docker0 root refcnt 2 
           
          $ docker container run --rm -p=3306:3306 --name mariadb-server -e MYSQL_ROOT_PASSWORD=secret -d mariadb:latest
           
          $ docker ps
          CONTAINER ID   IMAGE            COMMAND                  CREATED         STATUS         PORTS                                       NAMES
          2dc4fa93c179   mariadb:latest   "docker-entrypoint.s…"   4 minutes ago   Up 4 minutes   0.0.0.0:3306->3306/tcp, :::3306->3306/tcp   mariadb-server
           
          $ ./client/mysql -uroot -psecret -hlocalhost -P3306 --protocol=tcp
          MariaDB [(none)]> show databases;
          +--------------------+
          | Database           |
          +--------------------+
          | information_schema |
          | mysql              |
          | performance_schema |
          | sys                |
          +--------------------+
          4 rows in set (0.000 sec)
           
          $ ps aux|grep mysql
          999        2911  0.0  0.4 1408680 76784 ?       Ssl  10:51   0:00 mysqld
          


          Update:


          Obtained after setting again $ sudo tc qdisc add dev docker0 root netem delay 1000ms.
          Thanks, will take a look.

          anel Anel Husakovic added a comment - - edited Thanks, but still the same: $ tc qdisc show| grep docker qdisc noqueue 0: dev docker0 root refcnt 2   $ docker container run -- rm -p=3306:3306 --name mariadb-server -e MYSQL_ROOT_PASSWORD=secret -d mariadb:latest   $ docker ps CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES 2dc4fa93c179 mariadb:latest "docker-entrypoint.s…" 4 minutes ago Up 4 minutes 0.0.0.0:3306->3306 /tcp , :::3306->3306 /tcp mariadb-server   $ . /client/mysql -uroot -psecret -hlocalhost -P3306 --protocol=tcp MariaDB [(none)]> show databases; +--------------------+ | Database | +--------------------+ | information_schema | | mysql | | performance_schema | | sys | +--------------------+ 4 rows in set (0.000 sec)   $ ps aux| grep mysql 999 2911 0.0 0.4 1408680 76784 ? Ssl 10:51 0:00 mysqld Update: Obtained after setting again $ sudo tc qdisc add dev docker0 root netem delay 1000ms . Thanks, will take a look.
          anel Anel Husakovic made changes -
          Attachment screenshot-3.png [ 61923 ]

          Current behavior (without patch) is ok IMHO - no matter how many times you press ^C you will get that query is killed and without result. When you press single ^C you exit, what is expected.

          anel Anel Husakovic added a comment - Current behavior (without patch) is ok IMHO - no matter how many times you press ^C you will get that query is killed and without result. When you press single ^C you exit, what is expected.

          anel, that Ctrl-C or Ctrl-D, or Ctrl-Z does not matter that much. I just do not want to type "quit", and that's it . I need some Ctrl-Whatever (-no, not terminated by Enter) to quit fast. From your pictures, it does not seem that there is an option to do that, so we might as well ignore bashism and Linuxism on Windows, and retain the previous behavior

          wlad Vladislav Vaintroub added a comment - anel , that Ctrl-C or Ctrl-D, or Ctrl-Z does not matter that much. I just do not want to type "quit", and that's it . I need some Ctrl-Whatever (-no, not terminated by Enter) to quit fast. From your pictures, it does not seem that there is an option to do that, so we might as well ignore bashism and Linuxism on Windows, and retain the previous behavior

          anel, I wouldn't say that exiting on ^C is expected. In fact I totally agree with the original description:

          Control-c should not exit the client. It should cancel partial input or abort the current statement if there is one, but it should not do anything else otherwise.
          Control-d should exit the client if the input line is empty.

          serg Sergei Golubchik added a comment - anel , I wouldn't say that exiting on ^C is expected. In fact I totally agree with the original description: Control-c should not exit the client. It should cancel partial input or abort the current statement if there is one, but it should not do anything else otherwise. Control-d should exit the client if the input line is empty.
          serg Sergei Golubchik made changes -
          Assignee Sergei Golubchik [ serg ] Anel Husakovic [ anel ]
          Status In Review [ 10002 ] Stalled [ 10000 ]
          anel Anel Husakovic made changes -
          Fix Version/s 10.2 [ 14601 ]
          Fix Version/s 10.4 [ 22408 ]
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ]
          julien.fritsch Julien Fritsch made changes -
          Assignee Anel Husakovic [ anel ]
          anel Anel Husakovic made changes -
          Status Stalled [ 10000 ] Open [ 1 ]
          anel Anel Husakovic made changes -
          Status Open [ 1 ] Confirmed [ 10101 ]
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Vicențiu Ciorbaru [ cvicentiu ]
          Status Confirmed [ 10101 ] In Review [ 10002 ]
          anel Anel Husakovic made changes -
          Fix Version/s 10.4 [ 22408 ]
          Fix Version/s 10.3 [ 22126 ]
          anel Anel Husakovic made changes -
          cvicentiu Vicențiu Ciorbaru made changes -
          Assignee Vicențiu Ciorbaru [ cvicentiu ] Anel Husakovic [ anel ]
          Status In Review [ 10002 ] Stalled [ 10000 ]
          anel Anel Husakovic made changes -
          Assignee Anel Husakovic [ anel ] Vicențiu Ciorbaru [ cvicentiu ]
          Status Stalled [ 10000 ] In Review [ 10002 ]
          cvicentiu Vicențiu Ciorbaru added a comment - - edited

          wlad While I understand your request for having an easy way to "quit fast" on windows, I do not see any evidence in Windows land where Ctrl-C should do that. Windows PowerShell, as an interactive shell does not quit when issued a Ctrl-C.

          The only other signal that Windows handles as part of it's Console signal handlers is Ctrl-Break. MySQL coded Ctrl-Break to behave just like Ctrl-C (unix style). I've made a patch that causes the client to exit if you hit Ctrl-Break.

          Is that acceptable for you? I really would like to keep ourselves compatible between MySQL and our implementation with Ctrl-C.

          A patch that implements this, with a commit comment explaining exact behaviour can be found here:
          https://github.com/MariaDB/server/commit/8b16448e9417d44132dbac6622af83f64f25486d
          serg Can you please also double check the commit comment, to make sure you agree with the behaviour proposed? I would like to push this directly into 10.4, as a bug fix, not a "new feature". Given our last meetup in Helsinki, I think that this should be in line with community expectations, but I would like your sanity check.

          cvicentiu Vicențiu Ciorbaru added a comment - - edited wlad While I understand your request for having an easy way to "quit fast" on windows, I do not see any evidence in Windows land where Ctrl-C should do that. Windows PowerShell, as an interactive shell does not quit when issued a Ctrl-C. The only other signal that Windows handles as part of it's Console signal handlers is Ctrl-Break. MySQL coded Ctrl-Break to behave just like Ctrl-C (unix style). I've made a patch that causes the client to exit if you hit Ctrl-Break. Is that acceptable for you? I really would like to keep ourselves compatible between MySQL and our implementation with Ctrl-C. A patch that implements this, with a commit comment explaining exact behaviour can be found here: https://github.com/MariaDB/server/commit/8b16448e9417d44132dbac6622af83f64f25486d serg Can you please also double check the commit comment, to make sure you agree with the behaviour proposed? I would like to push this directly into 10.4, as a bug fix, not a "new feature". Given our last meetup in Helsinki, I think that this should be in line with community expectations, but I would like your sanity check.
          wlad Vladislav Vaintroub added a comment - - edited

          cvicentiu . CTRL-C ends a console program , by default, if you do not program any handlers for it. Is not this an evidence? Secondly, I would like to see a pointer to a public discussion where a Windowshead explains his opinions on how CTRL-C in "interactive programs" should work. I did not see it in the last 30 years, and I'd say there is no religious views in this community about it, unlike Linux-community, that forces its views on anything else. I , personally, found current behavior convenient, and am disturbed if someone is changing it,

          I encourage you to find the authoritative guidelines on command line application on Windows. Authoritative is from someone who actually uses Windows daily (I'm afraid there is not many option in MySQL and MariaDB land )

          wlad Vladislav Vaintroub added a comment - - edited cvicentiu . CTRL-C ends a console program , by default, if you do not program any handlers for it. Is not this an evidence? Secondly, I would like to see a pointer to a public discussion where a Windowshead explains his opinions on how CTRL-C in "interactive programs" should work. I did not see it in the last 30 years, and I'd say there is no religious views in this community about it, unlike Linux-community, that forces its views on anything else. I , personally, found current behavior convenient, and am disturbed if someone is changing it, I encourage you to find the authoritative guidelines on command line application on Windows. Authoritative is from someone who actually uses Windows daily (I'm afraid there is not many option in MySQL and MariaDB land )

          if there are no guidelines, we can see what other standard Windows console tools do.
          cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break.
          choice.exe exits on both, it doesn't distinguish between them either.

          If we must have a Ctrl-something to quit mariadbd, I'd suggest Ctrl-D then. It's totally non-Windows, no Windows tool does that, so likely no Windows user will press is and expect a specific outcome. And it's what we on Linux do, so, familiar.

          serg Sergei Golubchik added a comment - if there are no guidelines, we can see what other standard Windows console tools do. cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break. choice.exe exits on both, it doesn't distinguish between them either. If we must have a Ctrl-something to quit mariadbd , I'd suggest Ctrl-D then. It's totally non-Windows, no Windows tool does that, so likely no Windows user will press is and expect a specific outcome. And it's what we on Linux do, so, familiar.
          wlad Vladislav Vaintroub added a comment - - edited

          There are no guidelines, but there are existing users, who will find this change "disruptive and unintiutive" on Windows.. I'm a bona-finde user who uses this OS everyday, and our tools there, too. I also happen not to have a Break on my keyboard, and it seems that Break is now well hidden on keyboards. Ctrl-D does not work. Ctrl-Z does not work. I would rather not not write "quit". So, of all Windows users we have feedback so far, none found the old behavior disruptive, and the all vocal ones found this proposed change disruptive.

          wlad Vladislav Vaintroub added a comment - - edited There are no guidelines, but there are existing users, who will find this change "disruptive and unintiutive" on Windows.. I'm a bona-finde user who uses this OS everyday, and our tools there, too. I also happen not to have a Break on my keyboard, and it seems that Break is now well hidden on keyboards. Ctrl-D does not work. Ctrl-Z does not work. I would rather not not write "quit". So, of all Windows users we have feedback so far, none found the old behavior disruptive, and the all vocal ones found this proposed change disruptive.
          wlad Vladislav Vaintroub added a comment - - edited

          So here is the deal - can we do exactly nothing about it Windows in 10.4, or whatever old version it was predestined to be fixed. It would be impolite to break tools in the very last version before release is going EOL. In 10.11+ I'll file a followup, to break mysql on Ctrl-D or Ctrl-Z or both. Because this version already uses ReadConsoleW, making it possible to catch Ctrl-<something> for <something> that is not "C" (via last parameter, CONSOLE_READCONSOLE_CONTROL , which only works in Unicode "W" version)

          wlad Vladislav Vaintroub added a comment - - edited So here is the deal - can we do exactly nothing about it Windows in 10.4, or whatever old version it was predestined to be fixed. It would be impolite to break tools in the very last version before release is going EOL. In 10.11+ I'll file a followup, to break mysql on Ctrl-D or Ctrl-Z or both. Because this version already uses ReadConsoleW, making it possible to catch Ctrl-<something> for <something> that is not "C" (via last parameter, CONSOLE_READCONSOLE_CONTROL , which only works in Unicode "W" version)
          • agree about 10.4
          • I couldn't understand whether you agree with the suggestion 'Ctrl-C and Ctrl-Break do not exit the tool, Ctrl-D does", could you state that explicitly, please?
          serg Sergei Golubchik added a comment - agree about 10.4 I couldn't understand whether you agree with the suggestion 'Ctrl-C and Ctrl-Break do not exit the tool, Ctrl-D does", could you state that explicitly, please?

          Yes, Ctrl-D interrupts, Iood enough to me, although python uses Ctrl-Z on Windows some reason. I have no idea about Ctrl-Break, I can't generate it, so I do not care either way.

          Checked statement: "cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break." netsh does actually

          wlad Vladislav Vaintroub added a comment - Yes, Ctrl-D interrupts, Iood enough to me, although python uses Ctrl-Z on Windows some reason. I have no idea about Ctrl-Break, I can't generate it, so I do not care either way. Checked statement: "cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break." netsh does actually
          cvicentiu Vicențiu Ciorbaru made changes -
          issue.field.resolutiondate 2024-01-22 18:31:56.0 2024-01-22 18:31:55.966
          cvicentiu Vicențiu Ciorbaru made changes -
          Fix Version/s 10.4.33 [ 29516 ]
          Fix Version/s 10.5.24 [ 29517 ]
          Fix Version/s 10.6.17 [ 29518 ]
          Fix Version/s 10.11.7 [ 29519 ]
          Fix Version/s 11.0.5 [ 29520 ]
          Fix Version/s 11.1.4 [ 29024 ]
          Fix Version/s 11.2.3 [ 29521 ]
          Fix Version/s 11.3.2 [ 29522 ]
          Fix Version/s 11.4.1 [ 29523 ]
          Fix Version/s 10.4 [ 22408 ]
          Resolution Fixed [ 1 ]
          Status In Review [ 10002 ] Closed [ 6 ]

          Implemented this by only changing Linux behaviour for CTRL-C and adding some extra windows support for non-interactive signals sent by the OS during terminal close, log off or shut down, like in MySQL's client implementation.

          cvicentiu Vicențiu Ciorbaru added a comment - Implemented this by only changing Linux behaviour for CTRL-C and adding some extra windows support for non-interactive signals sent by the OS during terminal close, log off or shut down, like in MySQL's client implementation.

          People

            cvicentiu Vicențiu Ciorbaru
            nkovacs Nikola Kovacs
            Votes:
            2 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.