Ensure lock is released when job arguments are mutated

[jrichard]

I have jobs that enqueue arbitrary, dependent jobs. In doing so I mutated the original arguments to .perform.

class ExampleChainableJob
  def self.perform(job_class_chain)
    # do stuff

    next_job_class = job_class_chain.shift  # mutation of args!

    if next_job_class
      Resque.enqueue(next_job_class.constantize, job_class_chain)
    end
  end
end

This causes the lock key that gets generated to write the lock before my job runs (https://github.com/lantins/resque-lock-timeout/blob/master/lib/resque/plugins/lock_timeout.rb#L277) to NOT be the same as the key that gets generated to remove the lock when my job completes (https://github.com/lantins/resque-lock-timeout/blob/master/lib/resque/plugins/lock_timeout.rb#L295).

This results in the lock not being removed as expected.

Generally, mutating args is a little risky, but I think this is a good opportunity for #around_perform_lock ensure that it attempts to remove the same lock it created by remembering the key it used to create the lock and reuse it instead of regenerating the key.

Implementation wise, it seems like #acquire_lock_impl! will need to return the key it used to acquire the lock or take a key instead of lock_key_method. Do you have a preference for the approach? I'll work on a PR in coming days.

[lantins]

Thanks for the detailed analysis 💯

Like you said, having around_perform_lock remember what key it used to create the lock sounds good.

I'd rather see the fully formed key be passed into acquire_lock_impl! rather then changing what it returns, what it returns at the moment isn't fantastic.

In general, I think the key should be passed into a few more places, rather then generating it multiple times, but that's another issue...

I look forward to seeing the PR :)

[lantins]

One snag @jrichard :(

I've just merged #27 and that makes things more complicated when the args have been mutated before the DirtyExit is raised.

[lantins]

Closing due to time / the fact I made things more difficult back when.

Happy to receive a future PR, though I'd have to re-assess due to the long period of time that's passed.

I expect you've likely passed on though, and no worries, thanks for the contribution. Sorry for being slack.