Skip to content

Using multiple transaction blocks should enforce locking even if redundant #431

Description

@whitfin

The code block found here does not make sense:

  @spec transaction(Cachex.t(), [any], (-> any)) :: any
  def transaction(cache() = cache, keys, fun) when is_list(keys) do
    case transaction?() do
      true -> fun.()
      false -> Queue.transaction(cache, keys, fun)
    end
  end

Rather than allowing the function to run if we're already in a transaction, it should compare the keys to ensure the the current transaction has a superset of them locked. If it doesn't, this should raise an error because it's almost definitely a logic issue.

Although it's a very weird thing to do a transaction in a transaction (because it does nothing), you could lock a key in an outer transaction then try to lock a key in an inner transaction; it'd just work without actually locking anything. This is a bug, even if there's no reason to ever do this.

The better solution is to store the locked keys of the transaction inside Process.get(:cachex_transaction) rather than a boolean. Then we can compare within a transaction and error when appropriate.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions