From 2ac257fc098dc58bc44e78145dee8a45f2561b5c Mon Sep 17 00:00:00 2001
From: Leo Liu <sdl.web@gmail.com>
Date: Tue, 7 Nov 2023 15:01:22 +0800
Subject: [PATCH] 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.
---
 src/gproc_lib.erl  |  0
 src/gproc_pool.erl | 30 ++++++++----------------------
 2 files changed, 8 insertions(+), 22 deletions(-)
 mode change 100755 => 100644 src/gproc_lib.erl

diff --git a/src/gproc_lib.erl b/src/gproc_lib.erl
old mode 100755
new mode 100644
diff --git a/src/gproc_pool.erl b/src/gproc_pool.erl
index 709f323..be37c8d 100644
--- a/src/gproc_pool.erl
+++ b/src/gproc_pool.erl
@@ -439,8 +439,7 @@ claim(Pool, F, Wait) ->
     case gproc:get_value(?POOL(Pool), shared) of
 	{0, _} -> false;
 	{_, claim} ->
-	    W = setup_wait(Wait, Pool),
-	    claim_w(Pool, F, W);
+	    claim_w(Pool, F, setup_wait(Wait));
 	_ ->
 	    error(badarg)
     end.
@@ -452,7 +451,6 @@ claim_w(Pool, F, W) ->
 	false ->
 	    claim_w(Pool, F, do_wait(W));
 	Other ->
-	    clear_wait(W),
 	    Other
     end.
 
@@ -554,35 +552,23 @@ execute_claim(F, K, Pid) ->
         ?MAY_FAIL(gproc:reset_counter(K))
     end.
 
-setup_wait(nowait, _) ->
+setup_wait(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) ->
     timeout;
-do_wait({busy_wait, Ref} = W) ->
+do_wait({busy_wait, EndTime} = W) ->
     %% Yielding here serves two purposes:
     %% 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)
     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.
 
-clear_wait(nowait) ->
-    ok;
-clear_wait({busy_wait, Ref}) ->
-    erlang:cancel_timer(Ref),
-    ok.
-
 %% @spec log(GprocKey) -> integer()
 %% @doc Update a counter associated with a worker name.
 %%