Browse Source

Replace timer with monotonic time in gproc_pool (#195)

I have often observed stray timeout messages even after modifying
clear_wait/1 to flush out these messages like so:

clear_wait(nowait) ->
    ok;
clear_wait({busy_wait, Ref}) ->
    erlang:cancel_timer(Ref),
    receive {timeout, Ref, _} -> ok
    after 0 -> ok
    end.

The documentation of erlang:cancel_timer/2 has this to say:

  ...Even if the timer had expired, it does not tell you if the time-out
  message has arrived at its destination yet.

That means getting rid of unwanted timeout messages is racy and
expensive. In this change monotonic time is used to detect timeout
which avoids this problem.
Leo Liu 1 year ago
parent
commit
2ac257fc09
1 changed files with 8 additions and 22 deletions
  1. 8 22
      src/gproc_lib.erl

+ 8 - 22
src/gproc_lib.erl

@@ -439,8 +439,7 @@ claim(Pool, F, Wait) ->
     case gproc:get_value(?POOL(Pool), shared) of
     case gproc:get_value(?POOL(Pool), shared) of
 	{0, _} -> false;
 	{0, _} -> false;
 	{_, claim} ->
 	{_, claim} ->
-	    W = setup_wait(Wait, Pool),
-	    claim_w(Pool, F, W);
+	    claim_w(Pool, F, setup_wait(Wait));
 	_ ->
 	_ ->
 	    error(badarg)
 	    error(badarg)
     end.
     end.
@@ -452,7 +451,6 @@ claim_w(Pool, F, W) ->
 	false ->
 	false ->
 	    claim_w(Pool, F, do_wait(W));
 	    claim_w(Pool, F, do_wait(W));
 	Other ->
 	Other ->
-	    clear_wait(W),
 	    Other
 	    Other
     end.
     end.
 
 
@@ -554,35 +552,23 @@ execute_claim(F, K, Pid) ->
         ?MAY_FAIL(gproc:reset_counter(K))
         ?MAY_FAIL(gproc:reset_counter(K))
     end.
     end.
 
 
-setup_wait(nowait, _) ->
+setup_wait(nowait) ->
     nowait;
     nowait;
-setup_wait({busy_wait, MS}, Pool) ->
-    Ref = erlang:start_timer(MS, self(), {claim, Pool}),
-    {busy_wait, Ref}.
+setup_wait({busy_wait, MS}) ->
+    {busy_wait, erlang:monotonic_time(millisecond) + MS}.
 
 
 do_wait(nowait) ->
 do_wait(nowait) ->
     timeout;
     timeout;
-do_wait({busy_wait, Ref} = W) ->
+do_wait({busy_wait, EndTime} = W) ->
     %% Yielding here serves two purposes:
     %% Yielding here serves two purposes:
     %% 1) Increase the chance that whoever's before us can finish
     %% 1) Increase the chance that whoever's before us can finish
     %% 2) The value of read_timer/1 only refreshes after yield (so I've heard)
     %% 2) The value of read_timer/1 only refreshes after yield (so I've heard)
     erlang:yield(),
     erlang:yield(),
-    case erlang:read_timer(Ref) of
-	false ->
-            receive {timeout, Ref, _} -> ok
-            after 0 -> ok
-            end,
-	    timeout;
-	_ ->
-	    W
+    case erlang:monotonic_time(millisecond) >= EndTime of
+        true -> timeout;
+        false -> W
     end.
     end.
 
 
-clear_wait(nowait) ->
-    ok;
-clear_wait({busy_wait, Ref}) ->
-    erlang:cancel_timer(Ref),
-    ok.
-
 %% @spec log(GprocKey) -> integer()
 %% @spec log(GprocKey) -> integer()
 %% @doc Update a counter associated with a worker name.
 %% @doc Update a counter associated with a worker name.
 %%
 %%