Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on Application startup with String.to_existing_atom #85

Closed
arjan opened this issue Feb 7, 2024 · 6 comments · Fixed by #87
Closed

Crash on Application startup with String.to_existing_atom #85

arjan opened this issue Feb 7, 2024 · 6 comments · Fixed by #87
Assignees
Labels

Comments

@arjan
Copy link

arjan commented Feb 7, 2024

Hammer.Supervisor fails to start because it assumes the hammer backend names already exist in the atom pool.

Easily reproducible with running the following in iex:

Application.put_env(:hammer, :backend, [default: {Hammer.Backend.ETS, [expiry_ms: 10]}])
Mix.install([:hammer])

Crashes in the following way:

20:58:30.827 [notice] Application hammer exited: Hammer.Application.start(:normal, []) returned an error: an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

        :erlang.binary_to_existing_atom("hammer_backend_default_pool", :utf8)
        (hammer 6.2.0) lib/hammer/supervisor.ex:30: anonymous fn/1 in Hammer.Supervisor.init/1
        (elixir 1.15.4) lib/enum.ex:1693: Enum."-map/2-lists^map/1-1-"/2
        (hammer 6.2.0) lib/hammer/supervisor.ex:28: Hammer.Supervisor.init/1
        (stdlib 5.0.2) supervisor.erl:330: :supervisor.init/1
        (stdlib 5.0.2) gen_server.erl:962: :gen_server.init_it/2
        (stdlib 5.0.2) gen_server.erl:917: :gen_server.init_it/6
        (stdlib 5.0.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3
@arjan arjan added the bug label Feb 7, 2024
@epinault
Copy link
Contributor

hello! thanks for the report. What version oif Elixir and Erlang OTP are you running?

@arjan
Copy link
Author

arjan commented Feb 13, 2024

Otp 26, elixir 1.15

@epinault
Copy link
Contributor

epinault commented Feb 13, 2024

I see, yea I have not done any testing around that yet. it s related to the change they added in 1.15 . https://hexdocs.pm/elixir/1.15/changelog.html#potential-incompatibilities . Can you try this in your project and see if that helps?

@njwest
Copy link
Contributor

njwest commented Feb 20, 2024

+1, getting the same error on a fresh install of Hammer 6.2 on Elixir 1.15.6 and OTP 26

@njwest
Copy link
Contributor

njwest commented Feb 20, 2024

I see, yea I have not done any testing around that yet. it s related to the change they added in 1.15 . https://hexdocs.pm/elixir/1.15/changelog.html#potential-incompatibilities . Can you try this in your project and see if that helps?

Tried this, did not help. Looking at the code I don't see where these atoms are getting instantiated prior to String.to_existing_atom/1 call

@njwest
Copy link
Contributor

njwest commented Feb 20, 2024

Hammer.Supervisor fails to start because it assumes the hammer backend names already exist in the atom pool.

@arjan I am (edit:)ALMOST able to bypass this bug by changing my hammer backend: config value from a list to a single tuple like so:

config :hammer,
  backend: {
      Hammer.Backend.ETS,
      [
        expiry_ms: 60_000 * 60 * 2,
        cleanup_interval_ms: 300_000 * 2
      ]
    }

because single-backend config uses a single static default pool atom, HOWEVER when I do this I run into the following error when trying to call Hammer:

** (exit) an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

        :erlang.binary_to_existing_atom("hammer_backend_ets_pool", :utf8)
        (hammer 6.2.0) lib/hammer/utils.ex:9: Hammer.Utils.pool_name/1
        (hammer 6.2.0) lib/hammer.ex:255: Hammer.call_backend/3
        (hammer 6.2.0) lib/hammer.ex:51: Hammer.check_rate/4

new issue opened #88

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants