Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please make it shorter?

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

Expand Down
43 changes: 22 additions & 21 deletions libs/eavmlib/src/timer_manager.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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()}]
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -153,27 +159,22 @@ 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} ->
MgrPid ! {canceled, self()},
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.
29 changes: 29 additions & 0 deletions tests/libs/eavmlib/test_timer_manager.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down Expand Up @@ -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,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this comment. I know that some LLM writes them, but this is maintenance debt, once this is merged regression is gone.

%% 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this comment. I know that some LLM writes them, but this is maintenance debt, once this is merged regression is gone.

%% 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),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would force a schedule out before cancelling. "receive after 50 -> ok end" (we don't have erlang:yield/0 in this branch yet).

receive
should_not_arrive -> throw(timer_fired_despite_cancel)
after 500 -> ok
end,
ok.

wait_for_timeout(Msg, Timeout) ->
receive
{timeout, _TimerRef, Msg} ->
Expand Down
Loading