Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Update quickcheck to use blocking/1 and fix uncovered bug

Blocking/1 was added in quickcheck 1.27 and allows the test to indicate
which states might block execution and lets quickcheck find bugs around
them.

The bug uncovered has to do with when a process holding a worker exits,
the DOWN monitor fires and poolboy calls supervisor:terminate_child on
the worker the process held. When the EXIT message comes in for that
process, we then change the state around such that there is an
additional worker.

This caused a race condition if there were already checkout messages
waiting in the mailbox, because the EXIT message would come in behind
the checkout messages, and so even though we had just killed a worker,
we'd fail checkouts because we hadn't processed the EXIT yet.

The fix is to call into handle_worker_exit right after we terminate the
worker, thus updating our state to reflect the actual worker count
before processing any messages in the mailbox.
  • Loading branch information...
commit 3acb45739315e28174985d057c003686124e139b 1 parent 6294c7d
@Vagabond Vagabond authored
Showing with 38 additions and 25 deletions.
  1. +6 −1 src/poolboy.erl
  2. +32 −24 test/poolboy_eqc.erl
View
7 src/poolboy.erl
@@ -264,7 +264,12 @@ handle_info({'DOWN', Ref, _, _, _}, StateName, State) ->
[[Pid]] ->
Sup = State#state.supervisor,
ok = supervisor:terminate_child(Sup, Pid),
- {next_state, StateName, State};
+ %% Don't wait for the EXIT message to come in.
+ %% Deal with the worker exit right now to avoid
+ %% a race condition with messages waiting in the
+ %% mailbox.
+ true = ets:delete(State#state.monitors, Pid),
+ handle_worker_exit(Pid, StateName, State);
[] ->
{next_state, StateName, State}
end;
View
56 test/poolboy_eqc.erl
@@ -83,7 +83,8 @@ checkin(Pool, {Worker, _}) ->
kill_worker({Worker, _}) ->
exit(Worker, kill),
- timer:sleep(1).
+ timer:sleep(1),
+ Worker.
kill_idle_worker(Pool) ->
Pid = poolboy:checkout(Pool, false),
@@ -115,29 +116,36 @@ precondition(S,{call,_,checkin,[_Pool, Pid]}) ->
precondition(_S,{call,_,_,_}) ->
true.
-%% XXX comment out for parallel mode XXX
-%dynamic_precondition(S = #state{pid=Pid},_) when Pid /= undefined ->
- %State = if length(S#state.checked_out) == S#state.size + S#state.max_overflow ->
- %full;
- %length(S#state.checked_out) >= S#state.size ->
- %overflow;
- %true ->
- %ready
- %end,
-
- %Workers = max(0, S#state.size - length(S#state.checked_out)),
- %OverFlow = max(0, length(S#state.checked_out) - S#state.size),
- %Monitors = length(S#state.checked_out),
-
- %RealStatus = gen_fsm:sync_send_all_state_event(Pid, status),
- %case RealStatus == {State, Workers, OverFlow, Monitors} of
- %true ->
- %true;
- %_ ->
- %exit({wrong_state, RealStatus, {State, Workers, OverFlow, Monitors}})
- %end;
-%dynamic_precondition(_,_) ->
- %true.
+%% check model state against internal state, only used in sequential tests
+invariant(S = #state{pid=Pid},_) when Pid /= undefined ->
+ State = if length(S#state.checked_out) == S#state.size + S#state.max_overflow ->
+ full;
+ length(S#state.checked_out) >= S#state.size ->
+ overflow;
+ true ->
+ ready
+ end,
+
+ Workers = max(0, S#state.size - length(S#state.checked_out)),
+ OverFlow = max(0, length(S#state.checked_out) - S#state.size),
+ Monitors = length(S#state.checked_out),
+
+ RealStatus = gen_fsm:sync_send_all_state_event(Pid, status),
+ case RealStatus == {State, Workers, OverFlow, Monitors} of
+ true ->
+ true;
+ _ ->
+ {wrong_state, RealStatus, {State, Workers, OverFlow, Monitors}}
+ end;
+invariant(_,_) ->
+ true.
+
+%% what states block
+blocking(S, {call, _, checkout_block, _}) ->
+ %% blocking checkout can block if we expect a checkout to fail
+ not checkout_ok(S);
+blocking(_, _) ->
+ false.
postcondition(S,{call,_,checkout_block,[_Pool]},R) ->
case R of
Please sign in to comment.
Something went wrong with that request. Please try again.