Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Fixes cocaine path duplication bug #2169

Closed
wants to merge 2 commits into from

Conversation

christophed
Copy link
Contributor

If your path gets too big (e.g., you start appending your path thousands of times) you're going to have a bad time. On our boxes we noticed that paperclip would keep appending the Cocaine command line path, which crushes the CPU after some threshold.

Modified test to check for this and added a fix.

@@ -29,7 +29,7 @@
Paperclip.options[:command_path] = "/opt/my_app/bin"
Paperclip.run("convert", "stuff")
Paperclip.run("convert", "more_stuff")
assert_equal 1, [Cocaine::CommandLine.path].flatten.size
assert_equal 1, Cocaine::CommandLine.path.scan(Paperclip.options[:command_path]).count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [92/80]

@@ -24,7 +24,8 @@ def interpolates key, &block
#
def run(cmd, arguments = "", interpolation_values = {}, local_options = {})
command_path = options[:command_path]
Cocaine::CommandLine.path = [Cocaine::CommandLine.path, command_path].flatten.compact.uniq
cocaine_path_array = Cocaine::CommandLine.path.try(:split, Cocaine::OS.path_separator)
Cocaine::CommandLine.path = [cocaine_path_array, command_path].flatten.compact.uniq

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [89/80]

@tute tute closed this in f7aa320 Apr 12, 2016
@tute
Copy link
Contributor

tute commented Apr 12, 2016

Thank you! I squashed and merged into master, keeping your authorship. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants