diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2021-01-15 12:23:50 -0500 |
---|---|---|
committer | Nick Vatamaniuc <vatamane@apache.org> | 2021-01-15 12:45:46 -0500 |
commit | 3c51d954a0517cb21b6f36f98838b840f63746e2 (patch) | |
tree | 478b4fb59cff7c0d6dc848dd4f5644ccb86f6ceb | |
parent | ce1ca2c4e85720848622e1fbed2c67d6882e904f (diff) | |
download | couchdb-do_not_return_bad_os_processes_into_the_pool.tar.gz |
Do not return broken processes to the query process pooldo_not_return_bad_os_processes_into_the_pool
Previously, if an error was thrown in a `with_ddoc_proc/2` callback, the
process was still returned to the process pool in the `after` clause. However,
in some cases, for example when processing a _list response, the process might
end up stuck in a bad state state, such that it could not be re-used anymore.
In such a case, a subsequent user of that couch_js process would end up
throwing an error and crashing.
Fixes #2962
-rw-r--r-- | src/couch/src/couch_query_servers.erl | 11 | ||||
-rw-r--r-- | src/couch/test/eunit/couchdb_os_proc_pool.erl | 45 |
2 files changed, 52 insertions, 4 deletions
diff --git a/src/couch/src/couch_query_servers.erl b/src/couch/src/couch_query_servers.erl index 6649df364..c6d71345a 100644 --- a/src/couch/src/couch_query_servers.erl +++ b/src/couch/src/couch_query_servers.erl @@ -523,9 +523,14 @@ with_ddoc_proc(#doc{id=DDocId,revs={Start, [DiskRev|_]}}=DDoc, Fun) -> Rev = couch_doc:rev_to_str({Start, DiskRev}), DDocKey = {DDocId, Rev}, Proc = get_ddoc_process(DDoc, DDocKey), - try Fun({Proc, DDocId}) - after - ok = ret_os_process(Proc) + try Fun({Proc, DDocId}) of + Resp -> + ok = ret_os_process(Proc), + Resp + catch Tag:Err -> + Stack = erlang:get_stacktrace(), + catch proc_stop(Proc), + erlang:raise(Tag, Err, Stack) end. proc_prompt(Proc, Args) -> diff --git a/src/couch/test/eunit/couchdb_os_proc_pool.erl b/src/couch/test/eunit/couchdb_os_proc_pool.erl index 69f8051ad..8f732b905 100644 --- a/src/couch/test/eunit/couchdb_os_proc_pool.erl +++ b/src/couch/test/eunit/couchdb_os_proc_pool.erl @@ -20,9 +20,11 @@ setup() -> ok = couch_proc_manager:reload(), + meck:new(couch_os_process, [passthrough]), ok = setup_config(). teardown(_) -> + meck:unload(), ok. os_proc_pool_test_() -> @@ -39,7 +41,8 @@ os_proc_pool_test_() -> should_free_slot_on_proc_unexpected_exit(), should_reuse_known_proc(), % should_process_waiting_queue_as_fifo(), - should_reduce_pool_on_idle_os_procs() + should_reduce_pool_on_idle_os_procs(), + should_not_return_broken_process_to_the_pool() ] } } @@ -205,6 +208,46 @@ should_reduce_pool_on_idle_os_procs() -> end). +should_not_return_broken_process_to_the_pool() -> + ?_test(begin + config:set("query_server_config", + "os_process_soft_limit", "1", false), + ok = confirm_config("os_process_soft_limit", "1"), + + config:set("query_server_config", + "os_process_limit", "1", false), + ok = confirm_config("os_process_limit", "1"), + + DDoc = #doc{ + id = <<"ddoc1">>, + revs = {1, [<<"abc">>]}, + body = {[ + {<<"language">>, <<"javascript">>}, + {<<"views">>, {[ + {<<"v1">>, {[ + {<<"map">>, <<"function(doc) {emit(doc.value,1);}">>} + ]}} + ]}} + ]} + }, + meck:reset(couch_os_process), + + ?assertEqual(0, couch_proc_manager:get_proc_count()), + ok = couch_query_servers:with_ddoc_proc(DDoc, fun(_) -> ok end), + ?assertEqual(0, meck:num_calls(couch_os_process, stop, 1)), + ?assertEqual(1, couch_proc_manager:get_proc_count()), + + ?assertError(bad, couch_query_servers:with_ddoc_proc(DDoc, fun(_) -> + error(bad) + end)), + ?assertEqual(1, meck:num_calls(couch_os_process, stop, 1)), + + ok = couch_query_servers:with_ddoc_proc(DDoc, fun(_) -> ok end), + ?assertEqual(1, meck:num_calls(couch_os_process, stop, 1)), + ?assertEqual(1, couch_proc_manager:get_proc_count()) + end). + + setup_config() -> config:set("native_query_servers", "enable_erlang_query_server", "true", false), config:set("query_server_config", "os_process_limit", "3", false), |