Browse Source

Synchronous stop

Until now, the recommended way to stop a connection has been
`exit(Pid, normal)`. The behavior of `mysql:stop/1,3` differs
from what can be achieved with `exit/2` in that it is
synchronous, i.e. the calling process will block until it has
finished or timed out. (#111)
Jan Uhlig 6 years ago
parent
commit
fa1919efbd
6 changed files with 72 additions and 37 deletions
  1. 2 2
      README.md
  2. 47 1
      src/mysql.erl
  3. 9 12
      test/mysql_change_user_tests.erl
  4. 9 9
      test/mysql_tests.erl
  5. 1 9
      test/ssl_tests.erl
  6. 4 4
      test/transaction_tests.erl

+ 2 - 2
README.md

@@ -78,8 +78,8 @@ end
 {ok, [<<"SLEEP(5)">>], [[1]]} =
 {ok, [<<"SLEEP(5)">>], [[1]]} =
     mysql:query(Pid, <<"SELECT SLEEP(5)">>, 1000),
     mysql:query(Pid, <<"SELECT SLEEP(5)">>, 1000),
 
 
-%% Close the connection; make it exit normally
-exit(Pid, normal).
+%% Close the connection
+mysql:stop(Pid).
 ```
 ```
 
 
 Usage as a dependency
 Usage as a dependency

+ 47 - 1
src/mysql.erl

@@ -25,7 +25,8 @@
 %% gen_server is locally registered.
 %% gen_server is locally registered.
 -module(mysql).
 -module(mysql).
 
 
--export([start_link/1, query/2, query/3, query/4, query/5,
+-export([start_link/1, stop/1, stop/2,
+         query/2, query/3, query/4, query/5,
          execute/3, execute/4, execute/5,
          execute/3, execute/4, execute/5,
          prepare/2, prepare/3, unprepare/2,
          prepare/2, prepare/3, unprepare/2,
          warning_count/1, affected_rows/1, autocommit/1, insert_id/1,
          warning_count/1, affected_rows/1, autocommit/1, insert_id/1,
@@ -154,6 +155,51 @@ start_link(Options) ->
         _ -> ok
         _ -> ok
     end,
     end,
     Ret.
     Ret.
+ 
+%% @see stop/2.
+-spec stop(Conn) -> ok
+    when Conn :: connection().
+stop(Conn) ->
+    stop(Conn, infinity).
+
+%% @doc Stops a connection process and closes the connection. The
+%% process calling `stop' will be blocked until the connection
+%% process stops or the given timeout expires.
+%%
+%% If the connection is not stopped within the given timeout,
+%% an exit exception is raised with reason `timeout'.
+%%
+%% If the connection process exits with any other reason than `normal',
+%% an exit exception is raised with that reason.
+-spec stop(Conn, Timeout) -> ok
+    when Conn :: connection(),
+         Timeout :: timeout().
+stop(Conn, Timeout) ->
+    case erlang:function_exported(gen_server, stop, 3) of
+        true -> gen_server:stop(Conn, normal, Timeout);            %% OTP >= 18
+        false -> backported_gen_server_stop(Conn, normal, Timeout) %% OTP < 18
+    end.
+
+-spec backported_gen_server_stop(Conn, Reason, Timeout) -> ok
+    when Conn :: connection(),
+         Reason :: term(),
+         Timeout :: timeout().
+backported_gen_server_stop(Conn, Reason, Timeout) ->
+    Monitor=monitor(process, Conn),
+    exit(Conn, Reason),
+    receive
+        {'DOWN', Monitor, process, Conn, Reason} ->
+            ok;
+        {'DOWN', Monitor, process, Conn, UnexpectedReason} ->
+            exit(UnexpectedReason)
+    after Timeout ->
+        exit(Conn, kill),
+        receive
+            {'DOWN', Monitor, process, Conn, killed} ->
+                exit(timeout)
+        end
+    end.
+
 
 
 %% @see query/5.
 %% @see query/5.
 -spec query(Conn, Query) -> Result
 -spec query(Conn, Query) -> Result

+ 9 - 12
test/mysql_change_user_tests.erl

@@ -32,7 +32,7 @@ correct_credentials_test() ->
     Pid = connect_db(?user1),
     Pid = connect_db(?user1),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2)),
     ?assertEqual(ok, mysql:change_user(Pid, ?user2, ?password2)),
     ?assert(is_current_user(Pid, ?user2)),
     ?assert(is_current_user(Pid, ?user2)),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 %% Ensure that change user fails when given incorrect credentials,
 %% Ensure that change user fails when given incorrect credentials,
@@ -45,7 +45,7 @@ incorrect_credentials_fail_test() ->
     ExitReason = receive {'EXIT', Pid, Reason} -> Reason after 1000 -> error(timeout) end,
     ExitReason = receive {'EXIT', Pid, Reason} -> Reason after 1000 -> error(timeout) end,
     erlang:process_flag(trap_exit, TrapExit),
     erlang:process_flag(trap_exit, TrapExit),
     ?assertEqual(change_user_failed, ExitReason),
     ?assertEqual(change_user_failed, ExitReason),
-    close_conn(Pid),
+    ?assertExit(noproc, mysql:stop(Pid)),
     ok.
     ok.
 
 
 %% Ensure that user variables are reset after a successful change user
 %% Ensure that user variables are reset after a successful change user
@@ -59,7 +59,7 @@ reset_variables_test() ->
                   [<<"@foo">>],
                   [<<"@foo">>],
                   [[null]]},
                   [[null]]},
                  mysql:query(Pid, <<"SELECT @foo">>)),
                  mysql:query(Pid, <<"SELECT @foo">>)),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 %% Ensure that temporary tables are reset after a successful change user
 %% Ensure that temporary tables are reset after a successful change user
@@ -74,7 +74,7 @@ reset_temptables_test() ->
                   {1146, <<"42S02">>, _}},
                   {1146, <<"42S02">>, _}},
                  mysql:query(Pid, <<"SELECT * FROM otptest.foo">>)),
                  mysql:query(Pid, <<"SELECT * FROM otptest.foo">>)),
     ok = mysql:query(Pid, <<"DROP DATABASE IF EXISTS otptest">>),
     ok = mysql:query(Pid, <<"DROP DATABASE IF EXISTS otptest">>),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 %% Ensure that change user fails when inside an unmanaged transaction.
 %% Ensure that change user fails when inside an unmanaged transaction.
@@ -86,7 +86,7 @@ fail_in_unmanaged_transaction_test() ->
                  mysql:change_user(Pid, ?user2, ?password2)),
                  mysql:change_user(Pid, ?user2, ?password2)),
     ?assert(is_current_user(Pid, ?user1)),
     ?assert(is_current_user(Pid, ?user1)),
     ?assert(mysql:in_transaction(Pid)),
     ?assert(mysql:in_transaction(Pid)),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 %% Ensure that change user fails when inside a managed transaction.
 %% Ensure that change user fails when inside a managed transaction.
@@ -99,7 +99,7 @@ fail_in_managed_transaction_test() ->
                                                                ?password2)
                                                                ?password2)
                                    end)),
                                    end)),
     ?assert(is_current_user(Pid, ?user1)),
     ?assert(is_current_user(Pid, ?user1)),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 with_db_test() ->
 with_db_test() ->
@@ -112,7 +112,7 @@ with_db_test() ->
                   [[<<"otptest">>]]},
                   [[<<"otptest">>]]},
                  mysql:query(Pid, <<"SELECT DATABASE()">>)),
                  mysql:query(Pid, <<"SELECT DATABASE()">>)),
     ok = mysql:query(Pid, <<"DROP DATABASE IF EXISTS otptest">>),
     ok = mysql:query(Pid, <<"DROP DATABASE IF EXISTS otptest">>),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 execute_queries_test() ->
 execute_queries_test() ->
@@ -123,7 +123,7 @@ execute_queries_test() ->
                   [<<"@foo">>],
                   [<<"@foo">>],
                   [[123]]},
                   [[123]]},
                  mysql:query(Pid, <<"SELECT @foo">>)),
                  mysql:query(Pid, <<"SELECT @foo">>)),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 prepare_statements_test() ->
 prepare_statements_test() ->
@@ -134,7 +134,7 @@ prepare_statements_test() ->
                   [<<"foo">>],
                   [<<"foo">>],
                   [[123]]},
                   [[123]]},
                  mysql:execute(Pid, foo, [123])),
                  mysql:execute(Pid, foo, [123])),
-    close_conn(Pid),
+    mysql:stop(Pid),
     ok.
     ok.
 
 
 
 
@@ -143,9 +143,6 @@ connect_db(User) ->
                                   {log_warnings, false}]),
                                   {log_warnings, false}]),
     Pid.
     Pid.
 
 
-close_conn(Pid) ->
-    exit(Pid, normal).
-
 is_current_user(Pid, User) when is_binary(User) ->
 is_current_user(Pid, User) when is_binary(User) ->
     {ok, [<<"CURRENT_USER()">>], [[CurUser]]}=mysql:query(Pid, <<"SELECT CURRENT_USER()">>),
     {ok, [<<"CURRENT_USER()">>], [[CurUser]]}=mysql:query(Pid, <<"SELECT CURRENT_USER()">>),
     <<User/binary, "@localhost">> =:= CurUser;
     <<User/binary, "@localhost">> =:= CurUser;

+ 9 - 9
test/mysql_tests.erl

@@ -81,7 +81,7 @@ common_basic_check(ExtraOpts) ->
 common_conn_close() ->
 common_conn_close() ->
     Pid = whereis(tardis),
     Pid = whereis(tardis),
     process_flag(trap_exit, true),
     process_flag(trap_exit, true),
-    exit(Pid, normal),
+    mysql:stop(Pid),
     receive
     receive
         {'EXIT', Pid, normal} -> ok
         {'EXIT', Pid, normal} -> ok
     after
     after
@@ -94,10 +94,10 @@ exit_normal_test() ->
     {ok, Pid} = mysql:start_link(Options),
     {ok, Pid} = mysql:start_link(Options),
     {ok, ok, LoggedErrors} = error_logger_acc:capture(fun () ->
     {ok, ok, LoggedErrors} = error_logger_acc:capture(fun () ->
         %% Stop the connection without noise, errors or messages
         %% Stop the connection without noise, errors or messages
-        exit(Pid, normal),
+        mysql:stop(Pid),
         receive
         receive
             UnexpectedExitMessage -> UnexpectedExitMessage
             UnexpectedExitMessage -> UnexpectedExitMessage
-        after 50 ->
+        after 0 ->
             ok
             ok
         end
         end
     end),
     end),
@@ -171,7 +171,7 @@ keep_alive_test() ->
      ExpectedPrefix = io_lib:format("** Generic server ~p terminating", [Pid]),
      ExpectedPrefix = io_lib:format("** Generic server ~p terminating", [Pid]),
      ?assert(lists:prefix(lists:flatten(ExpectedPrefix), LoggedMsg)),
      ?assert(lists:prefix(lists:flatten(ExpectedPrefix), LoggedMsg)),
      ?assertMatch({crash_report, _}, LoggedReport),
      ?assertMatch({crash_report, _}, LoggedReport),
-     exit(Pid, normal).
+     ?assertExit(noproc, mysql:stop(Pid)).
 
 
 %% For R16B where sys:get_state/1 is not available.
 %% For R16B where sys:get_state/1 is not available.
 get_state(Process) ->
 get_state(Process) ->
@@ -193,7 +193,7 @@ query_test_() ->
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
-         exit(Pid, normal)
+         mysql:stop(Pid)
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          [{"Select db on connect", fun () -> connect_with_db(Pid) end},
          [{"Select db on connect", fun () -> connect_with_db(Pid) end},
@@ -222,7 +222,7 @@ connect_with_db(_Pid) ->
                                   {database, "otptest"}]),
                                   {database, "otptest"}]),
     ?assertMatch({ok, _, [[<<"otptest">>]]},
     ?assertMatch({ok, _, [[<<"otptest">>]]},
                  mysql:query(Pid, "SELECT DATABASE()")),
                  mysql:query(Pid, "SELECT DATABASE()")),
-    exit(Pid, normal).
+    mysql:stop(Pid).
 
 
 log_warnings_test() ->
 log_warnings_test() ->
     {ok, Pid} = mysql:start_link([{user, ?user}, {password, ?password}]),
     {ok, Pid} = mysql:start_link([{user, ?user}, {password, ?password}]),
@@ -244,7 +244,7 @@ log_warnings_test() ->
                  " in INSeRT INtO foo () VaLUeS ()\n", Log2),
                  " in INSeRT INtO foo () VaLUeS ()\n", Log2),
     ?assertEqual("Warning 1364: Field 'x' doesn't have a default value\n"
     ?assertEqual("Warning 1364: Field 'x' doesn't have a default value\n"
                  " in INSERT INTO foo () VALUES ()\n", Log3),
                  " in INSERT INTO foo () VALUES ()\n", Log3),
-    exit(Pid, normal).
+    mysql:stop(Pid).
 
 
 autocommit(Pid) ->
 autocommit(Pid) ->
     ?assert(mysql:autocommit(Pid)),
     ?assert(mysql:autocommit(Pid)),
@@ -722,7 +722,7 @@ timeout_test_() ->
          Pid
          Pid
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
-         exit(Pid, normal)
+         mysql:stop(Pid)
      end,
      end,
      {with, [fun (Pid) ->
      {with, [fun (Pid) ->
                  %% SLEEP was added in MySQL 5.0.12
                  %% SLEEP was added in MySQL 5.0.12
@@ -762,7 +762,7 @@ with_table_foo_test_() ->
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
-         exit(Pid, normal)
+         mysql:stop(Pid)
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          [{"Prepared statements", fun () -> prepared_statements(Pid) end},
          [{"Prepared statements", fun () -> prepared_statements(Pid) end},

+ 1 - 9
test/ssl_tests.erl

@@ -47,12 +47,4 @@ common_basic_check(ExtraOpts) ->
 
 
 common_conn_close() ->
 common_conn_close() ->
     Pid = whereis(tardis),
     Pid = whereis(tardis),
-    process_flag(trap_exit, true),
-    exit(Pid, normal),
-    receive
-        {'EXIT', Pid, normal} -> ok
-    after
-        5000 -> error({cant_stop_connection, Pid})
-    end,
-    process_flag(trap_exit, false).
-
+    mysql:stop(Pid, 5000).

+ 4 - 4
test/transaction_tests.erl

@@ -38,7 +38,7 @@ single_connection_test_() ->
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
          ok = mysql:query(Pid, <<"DROP DATABASE otptest">>),
-         exit(Pid, normal)
+         mysql:stop(Pid)
      end,
      end,
      fun (Pid) ->
      fun (Pid) ->
          [{"Simple atomic",        fun () -> simple_atomic(Pid) end},
          [{"Simple atomic",        fun () -> simple_atomic(Pid) end},
@@ -118,7 +118,7 @@ application_process_kill() ->
     ?assertMatch({ok, _, []},
     ?assertMatch({ok, _, []},
                  mysql:query(Pid2, <<"SELECT * from foo where bar = 42">>)),
                  mysql:query(Pid2, <<"SELECT * from foo where bar = 42">>)),
     ok = mysql:query(Pid2, <<"DROP DATABASE otptest">>),
     ok = mysql:query(Pid2, <<"DROP DATABASE otptest">>),
-    exit(Pid2, normal).
+    mysql:stop(Pid2).
 
 
 simple_atomic(Pid) ->
 simple_atomic(Pid) ->
     ?assertNot(mysql:in_transaction(Pid)),
     ?assertNot(mysql:in_transaction(Pid)),
@@ -207,8 +207,8 @@ deadlock_test_() ->
      end,
      end,
      fun ({Conn1, Conn2}) ->
      fun ({Conn1, Conn2}) ->
          ok = mysql:query(Conn1, <<"DROP DATABASE otptest">>, 1000),
          ok = mysql:query(Conn1, <<"DROP DATABASE otptest">>, 1000),
-         exit(Conn1, normal),
-         exit(Conn2, normal)
+         mysql:stop(Conn1),
+         mysql:stop(Conn2)
      end,
      end,
      fun (Conns) ->
      fun (Conns) ->
          [{"Plain queries", fun () -> deadlock_plain_queries(Conns) end},
          [{"Plain queries", fun () -> deadlock_plain_queries(Conns) end},