diff --git a/CHANGELOG.md b/CHANGELOG.md index 29b81ae4e4..6fccdc3c40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed several underallocation issues that could trigger data corruption on `binary:replace`, `zlib:compress` and bsd socket recv code. - Fixed a bug where `catch` would raise on regular atom results - Fixed ESP32 socket driver holding the global socket-list lock across blocking TCP connects, leaking the port on connect failure, losing concurrent `accept` waiters, leaking `netbuf` on receive error paths, and a recycled-`netconn` race between socket close and the event handler +- Fixed `timer_manager:send_after/3` (and therefore `erlang:send_after/3` and `timer:send_after/2,3`) + returning a reference that was never registered with the timer manager, so `cancel_timer/1` always + returned `false` and the scheduled message was delivered regardless of cancellation ## [0.7.0-alpha.1] - 2026-04-06 diff --git a/libs/eavmlib/src/timer_manager.erl b/libs/eavmlib/src/timer_manager.erl index 6154c94c52..aa239e87ac 100644 --- a/libs/eavmlib/src/timer_manager.erl +++ b/libs/eavmlib/src/timer_manager.erl @@ -25,7 +25,7 @@ -export([start/0, start_timer/3, cancel_timer/1, get_timer_refs/0, send_after/3]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2]). --export([run_timer/5, send_after_timer/3]). +-export([run_timer/6]). -record(state, { timers = [] :: [{reference(), pid()}] @@ -57,10 +57,11 @@ maybe_start() -> TimerRef :: reference(). start_timer(Time, Dest, Msg) -> maybe_start(), - gen_server:call(?SERVER_NAME, {Time, Dest, Msg}). + gen_server:call(?SERVER_NAME, {start_timer, Time, Dest, Msg}). %%----------------------------------------------------------------------------- -%% @param TimerRef a timer reference returned from {@link start_timer/3}. +%% @param TimerRef a timer reference returned from {@link start_timer/3} or +%% {@link send_after/3}. %% @returns the time in milliseconds left until the timer would have expired, %% or `false' if the timer was not found. %% @doc Cancel a timer. @@ -81,14 +82,16 @@ get_timer_refs() -> %% @param Dest Pid or server name to which to send the message. %% @param Msg Message to send to Dest after Time ms. %% @returns a reference that can be used to cancel the timer, if desired. -%% @doc Send Msg to Dest after Time ms. +%% @doc Send Msg to Dest after Time ms. Unlike {@link start_timer/3}, the +%% bare Msg is delivered (not wrapped in a `{timeout, Ref, Msg}' +%% tuple). The returned reference is registered with the timer +%% manager and can be cancelled with {@link cancel_timer/1}. %% @end %%----------------------------------------------------------------------------- -spec send_after(non_neg_integer(), pid() | atom(), term()) -> reference(). send_after(Time, Dest, Msg) -> - TimerRef = erlang:make_ref(), - spawn(?MODULE, send_after_timer, [Time, Dest, Msg]), - TimerRef. + maybe_start(), + gen_server:call(?SERVER_NAME, {send_after, Time, Dest, Msg}). %% %% ?GEN_SERVER callbacks @@ -111,8 +114,11 @@ handle_call({cancel, TimerRef}, From, #state{timers = Timers} = State) -> NewTimers = lists:keyreplace(Pid, 2, Timers, {{canceled, From}, Pid}), {noreply, State#state{timers = NewTimers}} end; -handle_call({Time, Dest, Msg}, _From, #state{timers = Timers} = State) -> - {TimerRef, Pid} = do_start_timer(Time, Dest, Msg), +handle_call({start_timer, Time, Dest, Msg}, _From, #state{timers = Timers} = State) -> + {TimerRef, Pid} = do_start_timer(Time, Dest, Msg, wrapped), + {reply, TimerRef, State#state{timers = [{TimerRef, Pid} | Timers]}}; +handle_call({send_after, Time, Dest, Msg}, _From, #state{timers = Timers} = State) -> + {TimerRef, Pid} = do_start_timer(Time, Dest, Msg, bare), {reply, TimerRef, State#state{timers = [{TimerRef, Pid} | Timers]}}. %% @hidden @@ -153,13 +159,13 @@ terminate(_Reason, _State) -> %% internal functions %% @private -do_start_timer(Time, Dest, Msg) -> +do_start_timer(Time, Dest, Msg, Mode) -> TimerRef = erlang:make_ref(), - Pid = spawn(?MODULE, run_timer, [self(), Time, TimerRef, Dest, Msg]), + Pid = spawn(?MODULE, run_timer, [self(), Time, TimerRef, Dest, Msg, Mode]), {TimerRef, Pid}. %% @private -run_timer(MgrPid, Time, TimerRef, Dest, Msg) -> +run_timer(MgrPid, Time, TimerRef, Dest, Msg, Mode) -> Start = erlang:system_time(millisecond), receive {cancel, From} -> @@ -167,13 +173,8 @@ run_timer(MgrPid, Time, TimerRef, Dest, Msg) -> gen_server:reply(From, Time - (erlang:system_time(millisecond) - Start)) after Time -> MgrPid ! {fired, self()}, - Dest ! {timeout, TimerRef, Msg} - end. - -%% @private -send_after_timer(Time, Dest, Msg) -> - TimerRef = start_timer(Time, self(), Msg), - receive - {timeout, TimerRef, Msg} -> - Dest ! Msg + case Mode of + wrapped -> Dest ! {timeout, TimerRef, Msg}; + bare -> Dest ! Msg + end end. diff --git a/tests/libs/eavmlib/test_timer_manager.erl b/tests/libs/eavmlib/test_timer_manager.erl index edb9501fcc..1af6b0a784 100644 --- a/tests/libs/eavmlib/test_timer_manager.erl +++ b/tests/libs/eavmlib/test_timer_manager.erl @@ -28,6 +28,8 @@ test() -> ok = test_cancel_timer_after_expiry(), ok = test_erlang_cancel_timer(), pong = test_send_after(), + ok = test_cancel_send_after(), + ok = test_cancel_send_after_suppresses_message(), ok. -include("etest.hrl"). @@ -73,6 +75,33 @@ test_send_after() -> timer_manager:send_after(100, self(), ping), pong = wait_for_timeout(ping, 5000). +%% Regression: the reference returned by send_after/3 must be a registered, +%% cancellable timer. It previously returned a throwaway ref that was never +%% in the timer table, so cancel_timer/1 always returned false. +test_cancel_send_after() -> + ?ASSERT_MATCH(timer_manager:get_timer_refs(), []), + TimerRef = timer_manager:send_after(60000, self(), test_cancel_send_after), + ?ASSERT_MATCH(timer_manager:get_timer_refs(), [TimerRef]), + R = timer_manager:cancel_timer(TimerRef), + ?ASSERT_TRUE(is_integer(R)), + ?ASSERT_TRUE(R > 0), + ?ASSERT_MATCH(timer_manager:get_timer_refs(), []), + R2 = timer_manager:cancel_timer(TimerRef), + ?ASSERT_EQUALS(false, R2), + ok. + +%% Regression: cancelling a send_after/3 timer must actually stop the message +%% from being delivered. It previously fired regardless of the cancel. +test_cancel_send_after_suppresses_message() -> + ?ASSERT_MATCH(timer_manager:get_timer_refs(), []), + TimerRef = timer_manager:send_after(100, self(), should_not_arrive), + _ = timer_manager:cancel_timer(TimerRef), + receive + should_not_arrive -> throw(timer_fired_despite_cancel) + after 500 -> ok + end, + ok. + wait_for_timeout(Msg, Timeout) -> receive {timeout, _TimerRef, Msg} ->