You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by da...@apache.org on 2018/09/05 21:27:13 UTC
[couchdb] branch fix-couch-server-race-condition created (now
8fb71a4)
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a change to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git.
at 8fb71a4 Reproduce race condition in couch_server
This branch includes the following new commits:
new 47c4fc4 Fix couch_server:terminate/2
new 8fb71a4 Reproduce race condition in couch_server
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[couchdb] 01/02: Fix couch_server:terminate/2
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 47c4fc49304779634409e79635826b5ab7ca49f3
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Wed Sep 5 16:24:17 2018 -0500
Fix couch_server:terminate/2
If couch_server terminates while there is an active open_async process
it will throw a function_clause exception because `couch_db:get_pid/1`
will fail due to the `#entry.db` member being undefined. Simple fix is
to just filter those out.
---
src/couch/src/couch_server.erl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/couch/src/couch_server.erl b/src/couch/src/couch_server.erl
index ede8227..5f5054c 100644
--- a/src/couch/src/couch_server.erl
+++ b/src/couch/src/couch_server.erl
@@ -244,7 +244,9 @@ terminate(Reason, Srv) ->
[Reason,
Srv#server{lru = redacted}]),
ets:foldl(fun(#entry{db = Db}, _) ->
- couch_util:shutdown_sync(couch_db:get_pid(Db))
+ if Db == undefined -> ok; true ->
+ couch_util:shutdown_sync(couch_db:get_pid(Db))
+ end
end, nil, couch_dbs),
ok.
[couchdb] 02/02: Reproduce race condition in couch_server
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 8fb71a4944b80815a18da7e5fd047049209ea843
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Wed Sep 5 16:25:41 2018 -0500
Reproduce race condition in couch_server
A rather uncommon bug found in production. Will write more as this is
just for show and tell.
For now this test case just demonstrates the issue that was discovered.
A fix is still being pondered.
---
src/couch/test/couch_server_tests.erl | 193 ++++++++++++++++++++++++++++++++++
1 file changed, 193 insertions(+)
diff --git a/src/couch/test/couch_server_tests.erl b/src/couch/test/couch_server_tests.erl
index a79f20f..95d340d 100644
--- a/src/couch/test/couch_server_tests.erl
+++ b/src/couch/test/couch_server_tests.erl
@@ -14,6 +14,7 @@
-include_lib("couch/include/couch_eunit.hrl").
-include_lib("couch/include/couch_db.hrl").
+-include("../src/couch_server_int.hrl").
start() ->
Ctx = test_util:start_couch(),
@@ -105,3 +106,195 @@ bad_engine_option_test_() ->
t_bad_engine_option() ->
Resp = couch_server:create(?tempdb(), [{engine, <<"cowabunga!">>}]),
?assertEqual(Resp, {error, {invalid_engine_extension, <<"cowabunga!">>}}).
+
+
+interleaved_requests_test_() ->
+ {
+ setup,
+ fun start_interleaved/0,
+ fun stop_interleaved/1,
+ fun make_interleaved_requests/1
+ }.
+
+
+start_interleaved() ->
+ TestDbName = ?tempdb(),
+ meck:new(couch_db, [passthrough]),
+ meck:expect(couch_db, start_link, fun(Engine, DbName, Filename, Options) ->
+ case DbName of
+ TestDbName ->
+ receive
+ go -> ok
+ end,
+ Res = meck:passthrough([Engine, DbName, Filename, Options]),
+ % We're unlinking and sending a delayed
+ % EXIT signal so that we can mimic a specific
+ % message order in couch_server. On a test machine
+ % this is a big race condition which affects the
+ % ability to induce the bug.
+ case Res of
+ {ok, Db} ->
+ DbPid = couch_db:get_pid(Db),
+ unlink(DbPid),
+ Msg = {'EXIT', DbPid, killed},
+ erlang:send_after(2000, whereis(couch_server), Msg);
+ _ ->
+ ok
+ end,
+ Res;
+ _ ->
+ meck:passthrough([Engine, DbName, Filename, Options])
+ end
+ end),
+ {test_util:start_couch(), TestDbName}.
+
+
+stop_interleaved({Ctx, TestDbName}) ->
+ %couch_server:delete(TestDbName, [?ADMIN_CTX]),
+ meck:unload(),
+ test_util:stop_couch(Ctx).
+
+
+make_interleaved_requests({_, TestDbName}) ->
+ [
+ fun() -> t_interleaved_create_delete_open(TestDbName) end
+ ].
+
+
+t_interleaved_create_delete_open(DbName) ->
+ {CrtRef, DelRef, OpenRef} = {make_ref(), make_ref(), make_ref()},
+ CrtMsg = {'$gen_call', {self(), CrtRef}, {create, DbName, [?ADMIN_CTX]}},
+ DelMsg = {'$gen_call', {self(), DelRef}, {delete, DbName, [?ADMIN_CTX]}},
+ OpenMsg = {'$gen_call', {self(), OpenRef}, {open, DbName, [?ADMIN_CTX]}},
+
+ % Start our first instance that will succeed in
+ % an invalid state. Notice that the opener pid
+ % spawned by couch_server:open_async/5 will halt
+ % in our meck expect function waiting for a message.
+ %
+ % We're using raw message passing here so that we don't
+ % have to coordinate multiple processes for this test.
+ whereis(couch_server) ! CrtMsg,
+
+ {ok, OpenerPid} = get_opener_pid(DbName),
+
+ % We have to suspend couch_server so that we can enqueue
+ % our next requests and let the opener finish processing.
+ erlang:suspend_process(whereis(couch_server)),
+
+ % Since couch_server is suspend, this delete request won't
+ % be processed until after the opener has sent its
+ % successful open response via gen_server:call/3
+ whereis(couch_server) ! DelMsg,
+
+ % This open request will be in the queue after the
+ % delete request but before the gen_server:call/3
+ % message which will establish the mixed up state
+ % in the couch_dbs ets table
+ whereis(couch_server) ! OpenMsg,
+
+ % First release the opener pid so it can continue
+ % working while we tweak meck
+ OpenerPid ! go,
+
+ % Replace our expect call to meck so that the OpenMsg
+ % isn't blocked on the receive
+ meck:expect(couch_db, start_link, fun(Engine, DbName1, Filename, Options) ->
+ meck:passthrough([Engine, DbName1, Filename, Options])
+ end),
+
+ % Wait for the '$gen_call' message from OpenerPid to arrive
+ % in couch_server's mailbox
+ {ok, Db} = wait_for_call_message(OpenerPid),
+
+ % We have to suspend the database process so that its 'EXIT'
+ % signal does affect the state of the couch_dbs ets table
+ erlang:suspend_process(couch_db:get_pid(Db)),
+
+ % Now monitor and resume the couch_server and assert that
+ % couch_server does not crash while processing OpenMsg
+ CSRef = erlang:monitor(process, whereis(couch_server)),
+ receive
+ {'DOWN', CSRef, _, _, Reason0} ->
+ erlang:error({crashed, Reason0})
+ after 0 ->
+ ok
+ end,
+
+ erlang:resume_process(whereis(couch_server)),
+
+ % Now make sure we get our responses as expected.
+ % The create response is expected to return not_found
+ % due to the delete request canceling the async opener
+ % pid and sending not_found to all waiters unconditionally
+ receive
+ {CrtRef, not_found} ->
+ ok;
+ {'DOWN', CSRef, _, _, Reason1} ->
+ erlang:error({crashed, Reason1})
+ after 5000 ->
+ erlang:error({timeout, create_message})
+ end,
+
+ timer:sleep(2000),
+
+ receive
+ {DelRef, ok} ->
+ ok;
+ {'DOWN', CSRef, _, _, Reason2} ->
+ erlang:error({crashed, Reason2})
+ after 5000 ->
+ erlang:error({timeout, delete_message})
+ end,
+
+ % The open request will actually get the {ok, Db}
+ % response from the initial create request
+ receive
+ {OpenRef, {ok, _}} ->
+ ok;
+ {'DOWN', CSRef, _, _, Reason3} ->
+ erlang:error({crashed, Reason3});
+ Else ->
+ {_, OtherMessages} = process_info(self(), messages),
+ erlang:error({wtf, [Else | OtherMessages]})
+ after 5000 ->
+ erlang:error({timeout, open_message})
+ end,
+
+ % And finally assert that couch_server is still
+ % alive.
+ ?assert(is_process_alive(whereis(couch_server))),
+ receive
+ {'DOWN', CSRef, _, _, Reason4} ->
+ erlang:error({crashed, Reason4})
+ after 0 ->
+ ok
+ end.
+
+
+get_opener_pid(DbName) ->
+ WaitFun = fun() ->
+ case ets:lookup(couch_dbs, DbName) of
+ [#entry{pid = Pid}] ->
+ {ok, Pid};
+ [] ->
+ wait
+ end
+ end,
+ test_util:wait(WaitFun).
+
+
+wait_for_call_message(Pid) ->
+ WaitFun = fun() ->
+ {_, Messages} = erlang:process_info(whereis(couch_server), messages),
+ Ret = lists:foldl(fun(Msg, Acc) ->
+ case Msg of
+ {'$gen_call', {Pid, _}, {open_result, _, _, {ok, _} = Resp}} ->
+ Resp;
+ _ ->
+ Acc
+ end
+ end, undefined, Messages),
+ if Ret == undefined -> wait; true -> Ret end
+ end,
+ test_util:wait(WaitFun).