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

Race condition on @on_load callback #792

Closed
josevalim opened this issue Jul 13, 2023 · 3 comments
Closed

Race condition on @on_load callback #792

josevalim opened this issue Jul 13, 2023 · 3 comments
Labels

Comments

@josevalim
Copy link
Contributor

Describe the bug

Initially reported here: elixir-lang/elixir#12777

This code:

https://github.com/swoosh/swoosh/blob/v1.11.3/lib/swoosh/mailer.ex#L236-L256

Uses Code.ensure_loaded/1 which is not sufficient if the adapter module is defined within the same application as the mailer module, as Code.ensure_loaded/1 does not wait for the module to be compiled, so depending on the order compilation goes, the check may fail.

Steps to Reproduce the Bug or Issue

It is hard to reproduce since it is a race condition but we have isolated the root case.

Expected behavior

One quick fix is to use Code.ensure_compiled/1. However, my suggestion is to move the @on_load check altogether to compile-time (via @before_compile/@after_compile callbacks) and use Code.ensure_compiled there. I don't see the need to check for this at runtime. The way Elixir manages your dependencies, you should assume that if a module is available at compile-time, it will be available at runtime.

Thank you!

Your reproducible example

No response

Screenshots or Videos

No response

Platform

It happens in any Erlang/OTP and Elixir version, but some versions may be more likely than others depending on how fast compilation goes.

Additional context

Thank you! ❤️

@princemaple
Copy link
Member

I love that making the compiler faster exposes a race condition.

@princemaple
Copy link
Member

Released 1.11.4 with Code.ensure_compiled/1. This should buy me time for the bigger refactor :)

@josevalim
Copy link
Contributor Author

Closing as explained in #793. Thanks!

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

No branches or pull requests

2 participants