
Race condition with temp_file?
Reported by Tammer Saleh | July 28th, 2007 @ 08:14 AM
I believe there is a race condition in the temp_file handling. Attachment_fu seems to save the path that comes in through uploaded_data=, which is a Tempfile path. I then get an exception of 'No such file or directory - /tmp/minimagic11345-24' when saving the AR record. Tracing the exception down leads me to this method:
def save_to_storage
if save_attachment?
- TODO: This overwrites the file if it exists, maybe have an allow_overwrite option?
FileUtils.mkdir_p(File.dirname(full_filename))
File.cp(temp_path, full_filename)
File.chmod(attachment_options[:chmod] || 0644, full_filename)
end
@old_filename = nil
true
end
Specifically, the exception happens in the File.cp call. The interesting thing is that save_attachment?
looks like this:
def save_attachment?
File.file?(temp_path.to_s)
end
Which means that temp_path
is valid inside @save_attachment?@, but invalid immediately afterward. My hunch is that this is due to a garbage collection race condition where the Tempfile (and associated file on disk) is being destroyed.
I'm sorry that I can't give you any more specific details than that, or a failing test, as the issue only seems to happen in about 1 in 500 saves.
Comments and changes to this ticket
-
Tammer Saleh July 28th, 2007 @ 08:14 AM
Sorry about the poor formatting. I assumed Textile (the formatting help link is giving me access denied).
-
Tammer Saleh July 28th, 2007 @ 08:14 AM
Another comment: I believe this method is supposed to guard against this, but may not be working:
- Gets the latest temp path from the collection of temp paths. While working with an attachment,
- multiple Tempfile objects may be created for various processing purposes (resizing, for example).
- An array of all the tempfile objects is stored so that the Tempfile instance is held on to until
- it's not needed anymore. The collection is cleared after saving the attachment.
def temp_path
p = temp_paths.first
p.respond_to?(:path) ? p.path : p.to_s
end
-
Tammer Saleh July 28th, 2007 @ 08:14 AM
I've come close to a solution. It seems that the following line in uploaded_data=
self.temp_path = file_data.path
Should actually be
self.temp_path = file_data
As temp_path() will call :path on the obj in the stack when it returns it. When you give it file_data.path, you're no longer holding onto the Tempfile object, which means garbage collection is free to remove it.
This has reduced the number of crashes to about 1 in 1500, but I'm still getting an exception at some point inside MiniMagickProcessor.process_attachment_with_processing.
-
Rick July 28th, 2007 @ 08:14 AM
- State changed from new to open
- Assigned user set to Rick
Crap, sorry about the formatting. I'll fix that in a LH update tonight. Use @@@ for codeblocks.
# use @@@ to start blah blah # now finish with @@@
It's textile-ish, but I left out a lot of the weird stuff it does. We're doing work here, we don't care about squiggly lines and apostrophes that look pretty!
Allright, I'll try incorporating your change. Wish I saw this while I was working on afu this afternoon...
-
Tammer Saleh July 28th, 2007 @ 08:14 AM
No prob. about the formatting.
Here's a stacktrace of the remaining race condition.
Errno::ENOENT: No such file or directory - /tmp/minimagic28373-2 /usr/local//lib/ruby/1.8/ftools.rb:59:in `stat' /usr/local//lib/ruby/1.8/ftools.rb:59:in `syscopy' /usr/local//lib/ruby/1.8/ftools.rb:89:in `cp' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu/backends/file_system_backend.rb:78:in `save_to_storage' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:359:in `after_process_attachment' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `send' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `callback' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `each' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `callback' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:243:in `create_or_update' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/base.rb:1539:in `save_without_validation!' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/validations.rb:762:in `save_without_transactions!' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:133:in `save!' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:59:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:95:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:121:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:133:in `save!' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:204:in `create_or_update_thumbnail' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/../../activesupport/lib/active_support/core_ext/object/misc.rb:23:in `returning' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:196:in `create_or_update_thumbnail' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:357:in `after_process_attachment' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:357:in `each' /home/builder/work/rule/sources/config/../vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:357:in `after_process_attachment' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `send' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `callback' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `each' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `callback' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/callbacks.rb:243:in `create_or_update' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/base.rb:1533:in `save_without_validation' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/validations.rb:752:in `save_without_transactions' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:129:in `save' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:59:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:95:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:121:in `transaction' /home/builder/work/rule/sources/config/../vendor/rails/activerecord/lib/active_record/transactions.rb:129:in `save' ./test/unit/photo_test.rb:27:in `__bind_1175605418_915240' /home/builder/work/rule/sources/config/../vendor/plugins/tb_test_helpers/lib/should.rb:51:in `call' /home/builder/work/rule/sources/config/../vendor/plugins/tb_test_helpers/lib/should.rb:51:in `test a new photo, given a file via uploaded_data= should create a file on assigning to uploaded_data' /home/builder/work/rule/sources/config/../vendor/plugins/tb_test_helpers/lib/should.rb:51:in `each' /home/builder/work/rule/sources/config/../vendor/plugins/tb_test_helpers/lib/should.rb:51:in `test a new photo, given a file via uploaded_data= should create a file on assigning to uploaded_data' /home/builder/work/rule/sources/config/../vendor/plugins/mocha-0.4.0/lib/mocha/test_case_adapter.rb:19:in `__send__' /home/builder/work/rule/sources/config/../vendor/plugins/mocha-0.4.0/lib/mocha/test_case_adapter.rb:19:in `run'
I have also been getting one that originates in copy_to_temp_file, which I'll send over when I next see it.
Hope this helps.
Tammer
-
Tim Lu July 28th, 2007 @ 08:14 AM
Hi guys,
I'm also getting a simlar error with attachment_fu but it only seems to happen once in a while also
[RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:330:in `size' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:330:in `set_size_from_temp_path' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `send' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `callback' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `each' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `callback' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:295:in `valid?' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/validations.rb:758:in `save_without_transactions!' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:133:in `save!' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:59:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:95:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:121:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:133:in `save!' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:206:in `create_or_update_thumbnail' [RAILS_ROOT]/vendor/rails/activesupport/lib/active_support/core_ext/object/misc.rb:23:in `returning' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:198:in `create_or_update_thumbnail' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:358:in `after_process_attachment' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:358:in `each' [RAILS_ROOT]/vendor/plugins/attachment_fu/lib/technoweenie/attachment_fu.rb:358:in `after_process_attachment' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `send' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:333:in `callback' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `each' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:330:in `callback' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/callbacks.rb:243:in `create_or_update' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/base.rb:1532:in `save_without_validation' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/validations.rb:749:in `save_without_transactions' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:129:in `save' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:59:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:95:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:121:in `transaction' [RAILS_ROOT]/vendor/rails/activerecord/lib/active_record/transactions.rb:129:in `save' [RAILS_ROOT]/app/controllers/user_controller.rb:44:in `new_user_image' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/base.rb:1070:in `send' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/base.rb:1070:in `perform_action_without_filters' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:635:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:438:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:438:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:438:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/plugins/acts_as_friendly_param/lib/friendly_filter.rb:26:in `filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:484:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:449:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:641:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:449:in `call' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:640:in `call_filter' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:622:in `perform_action_without_benchmark' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in `perform_action_without_rescue' /usr/lib64/ruby/1.8/benchmark.rb:293:in `measure' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in `perform_action_without_rescue' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/rescue.rb:81:in `perform_action' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/base.rb:427:in `send' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/base.rb:427:in `process_without_filters' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/filters.rb:627:in `process_without_session_management_support' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/session_management.rb:114:in `process' [RAILS_ROOT]/vendor/rails/actionpack/lib/action_controller/base.rb:330:in `process' [RAILS_ROOT]/vendor/rails/railties/lib/dispatcher.rb:41:in `dispatch' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:78:in `process' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:76:in `synchronize' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:76:in `process' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:618:in `process_client' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:617:in `each' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:617:in `process_client' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `initialize' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `new' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `initialize' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `new' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:271:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:270:in `each' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:270:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/bin/mongrel_rails:127:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/command.rb:211:in `run' /usr/lib64/ruby/gems/1.8/gems/mongrel-1.0.1/bin/mongrel_rails:243 /usr/bin/mongrel_rails:18:in `load' /usr/bin/mongrel_rails:18
Tim
-
Tim Lu July 28th, 2007 @ 08:14 AM
I think I figured out where the race conditions are coming from even after your fix, tammer, especially for users of minimagick.
1) the mini_magick library does not handle tempfiles correctly;
in the initialize function, it only saves the path of the tempfile
that is created - as a result, the tempfile can be lost by GC.
to fix this, do sometihng like this
def initialize(file_object) @file_object = file_object input_path = @file_object.path @path = input_path # Ensure that the file is an image run_command("identify #{@path}") end
http://rubyforge.org/tracker/ind...
2)
i'm not sure if this next fix is absolutely needed or not but in the resize_img
function in mini_magick_process.rb i changed
self.temp_path = img.path
to
self.temp_path = img
-
Tammer Saleh July 28th, 2007 @ 08:14 AM
Hi Tim,
Thanks a lot for finding the rest of the race conditions. Rick, do you have any idea how long it will take to get that change tested and released? Our CI server is killing us with this one :)
Thanks again,
Tammer
-
nickyp July 28th, 2007 @ 08:14 AM
I've used the information in here to try and solve another seemingly race-condition and tempfile garbage-collection bug.
Details here:
http://railsweenie.com/forums/3/topics/1459?page=1#posts-5313
I had to patch both mini_magick_processor.rb and mini_magick.rb itself.
-
Rick July 28th, 2007 @ 08:14 AM
I've committed your fix to attachment_fu. I'd suggest that you submit
the mini magick fix as a patch to the author of that gem. Feel free
to suggest a patch for attachment_fu that opens up the minimagick
class and makes the required fixes though.
-
Ben Lam July 28th, 2007 @ 08:14 AM
Not sure if this is related in anyway, but when using a_fu to upload large images, especially those that are beyond the max_size specified in the has_attachment call within the model I see a lot of these:
#<RangeError: 0xdbb51426 is recycled object> #<RangeError: 0xdbb5194e is recycled object> #<RangeError: 0xdbb5167e is recycled object> #<RangeError: 0xdbb51ba6 is recycled object>
scrolling past on my Webrick terminal. I can get up to 80 some odd reports of that RangeError if I'm using Ramon's add image via url method
For reference I've set my max size on uploads to 500KB and I'll be sure to see at least a handful these errors on any pic I try uploading (from my local disk) beyond that size, tons of that error if using Ramon's upload via URL method.
I can't seem to find much documentation on the cause of the RangeError except this following Eigenclass article article
Apologies if this has nothing to do with the subject of this thread.
-
Errol Siegel July 28th, 2007 @ 08:14 AM
In my case, attachment_fu seems to work for very small files (1-2Kb) but fails for anything bigger (even just 80-90Kb). In my case it fails every time.
I tried
#self.temp_path = file_data.path
self.temp_path = file_data
as suggested above
but this did not seem to change anything.
I did not look at the mini_magick stuff as I am not uploading image files.
Here is a trace:
Validation failed: Size is not included in the list
RAILS_ROOT: ./script/../config/..
Application Trace | Framework Trace | Full Trace
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/validations.rb:764:in `save_without_transactions!'
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/transactions.rb:133:in `save!'
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/connection_adapters/abstract/database_statements.rb:59:in `transaction'
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/transactions.rb:95:in `transaction'
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/transactions.rb:121:in `transaction'
C:/Users/errol.SERENGETI/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/transactions.rb:133:in `save!'
#{RAILS_ROOT}/app/controllers/support/attachment_controller.rb:10:in `new'
-
Rick July 28th, 2007 @ 08:14 AM
Please don't hijack this ticket with new issues, create new tickets for them. Your problem is a simple activerecord validation issue:
Validation failed: Size is not included in the list
Start there.
-
Errol Siegel July 28th, 2007 @ 08:14 AM
I'm sorry -- I didn't realize this was unrelated. I found a few other posts on various message boards describing the same symptoms I am seeing and they pointed me to this ticket.
Sorry to have bothered you.
-
Dean Mao July 28th, 2007 @ 08:14 AM
I also found that File.cp() is a bit funny... I had problems uploading large files with attachment_fu. The output file was getting randomly truncated between 50-200k even though the file itself was 4 megs.
The simple fix was to remove File.cp() in FileSystemBackend.save_to_storage with FileUtils.cp()
-
bitbutter July 28th, 2007 @ 08:14 AM
I'm also periodically getting both kinds of exception reported by Tammer and Tim Lu while using mini-magic as the image processor.
I tried to apply Tim Lu's suggested patch to a frozen version of mini-magic gem but i guess i bungled something since the result was that newly uploaded image thumbs were 'missing' (missing image icons) in my online app (no exceptions though)--but it worked as expected locally. For now i've set things back to the way they were when the exceptions were popping up from time to time.
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
People watching this ticket
Referenced by
-
22 Race condition with GC and Tempfile cleanup I found a similar problem to #6 but with more wide-rangi...