`without_versioning` not thread safe #326

Closed
dwbutler opened this Issue Feb 10, 2014 · 1 comment

Comments

Projects
None yet
2 participants
Contributor

dwbutler commented Feb 10, 2014

# Executes the given method or block without creating a new version.
def without_versioning(method = nil)
  paper_trail_was_enabled = self.paper_trail_enabled_for_model
  self.class.paper_trail_off
  method ? method.to_proc.call(self) : yield
ensure
  self.class.paper_trail_on if paper_trail_was_enabled
end

If multiple threads call this method at the same time, Bad Things will happen.

For example, versioning could be turned off for all threads, causing no threads to save any versions during the duration of this method call.

Or, one thread could have set self.paper_trail_enabled_for_model to false temporarily. Then, when without_versioning is called, paper_trail_was_enabled will be false. So versioning will be turned off permanently from that point forward.

The solution is probably to use a thread local variable for self.paper_trail_enabled_for_model. This will also cause self.class.paper_trail_off and self.class.paper_trail_on to be thread local.

Contributor

dwbutler commented Feb 11, 2014

To put this into context, I'm running my Rails app in JRuby under Torquebox, so thread safety is a real issue for me.

I'm working on a pull request for this that will use PaperTrail's paper_trail_store to track if Paper Trail is enabled on a particular model.

I believe this will also fix #307 by doing away with setting and unsetting the paper_trail_enabled_for_model class attribute (which apparently isn't thread safe).

batter added this to the 3.0.1 milestone Feb 20, 2014

batter self-assigned this Feb 20, 2014

batter closed this in 0359704 Feb 20, 2014

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