search

whitfin / cachex

A powerful caching library for Elixir with support for transactions, fallbacks and expirations
https://hexdocs.pm/cachex/
MIT License
1.6k stars 103 forks source link

Possible bug in :cachex_notify event handling? #325

Closed feld closed 9 months ago

feld commented 9 months ago

https://github.com/whitfin/cachex/blob/5b8983ef670539337373aac3fc6b5772ecfe9735/lib/cachex/hook.ex#L165-L182

I was trying to refactor this for a dialyzer error that seemed like it was mostly benign, but then I realized there may be a logic error here which (surprisingly) neither the compiler nor dialyzer picked up. Follow this logic:

  1. timeout() returns a non-nil value
  2. a Task is launched calling handle_notify/3
  3. If the Task yields in time, we have the case statement
  4. The task yielded a value! {:ok, {:ok, new_state}} -> {:noreply, state} ... but we returned the old state anyway just like if the Task didn't yield in time? Why?
whitfin commented 9 months ago

Yes, this is a bug - although like you say I highly doubt this ever affected anyone (and it dates back six years). Feel free to PR a fix!