Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serious memory leak #921

Closed
lustremedia opened this issue Aug 12, 2016 · 10 comments
Closed

Serious memory leak #921

lustremedia opened this issue Aug 12, 2016 · 10 comments

Comments

@lustremedia
Copy link

lustremedia commented Aug 12, 2016

When I scan my code, brakeman stops at Processing data flow in controllers and my memory flies out of the roof.

Using:
OS: Debian Stable (Bunsen Labs)
Ruby 2.3.1
Rails 4.2.1.7
Brakeman 3.3.3 and 3.3.4

$ brakeman
Loading scanner...
Processing application in /home/tb/00_projects/radio_ssatr
Processing gems...
[Notice] Detected Rails 4 application
Processing configuration...
[Notice] Escaping HTML by default
Parsing files...
Processing initializers...
Processing libs...ssed
Processing routes...          
Processing templates...       
Processing data flow in templates...
Processing models...          
Processing controllers...     
Processing data flow in controllers...
 1/21 controllers processed
@lustremedia
Copy link
Author

lustremedia commented Aug 12, 2016

$ brakeman --faster works without issues ...

@presidentbeef
Copy link
Owner

Please run with -d to see if you can narrow it down to a specific method being processed. Typically this happens when there are a large number of assignments to the same variable in branches. If you can share the code, that would be extremely helpful.

@lustremedia
Copy link
Author

lustremedia commented Sep 8, 2016

@presidentbeef I just revisited this issue. The leak is caused by my method below. Basically this is just a helper method to create certain html templates. (It is quite old code and I noticed that I don't use it anymore, but maybe it helps to fix a brakeman bug)

Once I remove this method brakeman works without issues ...

def lh_textlinks(ops={})
    ops.reverse_merge! text_1: nil, link_1: nil, text_2: nil, link_2: nil, text_3: nil, link_3: nil, id: nil, classes: nil
    c=""
    t3=""
    if ops[:text_3].present?
      if ops[:link_3].present?
        t3 << content_tag(:div, link_to( ops[:text_3].html_safe, ops[:link_3]), class: "small-medium-wrap")
      else
        t3 << content_tag(:div, content_tag(:p, ops[:text_3].html_safe), class: "small-medium-wrap")
      end

      # set clearfix before text1 link1
      if ops[:text_1].present?
        t3 << content_tag(:div,"", class: "clearfix")
      end
    end

    t1=""
    if ops[:text_1].present?
      if ops[:link_1].present?
        t1 << content_tag(:div, link_to(ops[:text_1].html_safe, ops[:link_1]), class: "big-medium-wrap")
      else
        t1 << content_tag(:div, content_tag(:p, ops[:text_1].html_safe), class: "big-medium-wrap")
      end
    end

    t2=""
    if ops[:text_2].present?
      # set clearfix after text1 link1
      if ops[:text_1].present?
       t2 << content_tag(:div,"", class: "clearfix")
      end

      if ops[:link_2].present?
        t2 << content_tag(:div, link_to(ops[:text_2].html_safe, ops[:link_2]), class: "big-regular-wrap")
      else
        t2 << content_tag(:div, content_tag(:p, ops[:text_2].html_safe), class: "big-regular-wrap")
      end
    end
    c << t3.html_safe
    c << t1.html_safe
    c << t2.html_safe
    content_tag :div, c.html_safe , class: "text-links-wrapper #{ops[:classes]}", id: ops[:id]
  end

@presidentbeef
Copy link
Owner

Perfect, thank you!

@wata727
Copy link

wata727 commented Sep 16, 2016

I have a same problem too.
For example, scaning https://github.com/opf/openproject ...

$ git clone --depth=1 git@github.com:opf/openproject.git
$ cd openproject
$ brakeman --debug
Loading scanner...
Processing application in /Users/wata727/workspace/openproject
Processing gems...
[Notice] Detected Rails 4 application
Processing configuration...
[Notice] Escaping HTML by default
Parsing files...

....

Processing Api::Experimental::QueriesController
Processing Api::Experimental::QueriesController#available_columns
Processing Api::Experimental::QueriesController#custom_field_filters
Processing Api::Experimental::QueriesController#grouped
Processing Api::Experimental::QueriesController#create  (freeze and memory leak!)
^C
Interrupted - exiting.
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:97:in `<<'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:31:in `block in deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `each'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp.rb:29:in `deep_clone'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:82:in `replace'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:70:in `process_default'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:77:in `block in process'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:112:in `in_context'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:71:in `process'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:61:in `block in process_default'
(eval):3:in `map!'
(eval):3:in `map!'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:59:in `process_default'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:102:in `process_call'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:75:in `block in process'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:112:in `in_context'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/ruby_parser/bm_sexp_processor.rb:71:in `process'
/Users/wata727/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/brakeman-3.4.0/lib/brakeman/processors/alias_processor.rb:61:in `block in process_default'

...

Environment

  • Brakeman 3.4.0
  • Ruby 2.3.1
  • OS X El Capitan 10.11.4

@presidentbeef
Copy link
Owner

@wata727 Thank you for the example. I will look into it. Please note the --faster option (or more specifically the --no-branching option) should help.

@wata727
Copy link

wata727 commented Sep 17, 2016

I have try with options such as --faster, --no-branching.

$ brakeman --faster
Loading scanner...
Processing application in /Users/watanabekazuma/workspace/ruby/openproject
Processing gems...
[Notice] Detected Rails 4 application
Processing configuration...
[Notice] Escaping HTML by default
Parsing files...
Processing initializers...
Processing libs...ssed
[Skipping]
Processing routes...
Processing templates...
Processing data flow in templates...
Processing models...
Processing controllers...
Processing data flow in controllers...
^C/86 controllers processed
Interrupted - exiting.
$ brakeman --no-branching
Loading scanner...
Processing application in /Users/watanabekazuma/workspace/ruby/openproject
Processing gems...
[Notice] Detected Rails 4 application
Processing configuration...
[Notice] Escaping HTML by default
Parsing files...
Processing initializers...
Processing libs...ssed
Processing routes...
Processing templates...
Processing data flow in templates...
Processing models...
Processing controllers...
Processing data flow in controllers...
^C/86 controllers processed
Interrupted - exiting.

Umm.. That's too bad...
Probably, this problem caused by deep_clone large Sexp objects. According to my research, it is such as parsed v3_params_as_internal method (https://github.com/opf/openproject/blob/dev/app/controllers/api/experimental/concerns/v3_naming.rb#L40)

In this method, params is assigned itself that parsed. This process is called repeatedly as the number of params. As a result, this method that parsed is too large Sexp objects.

@presidentbeef
Copy link
Owner

@lustremedia strange...I can't reproduce the issue with that method. Is it a view helper?

@lustremedia
Copy link
Author

@presidentbeef this method is located within my view-helper module (helper class) of a rails project

@presidentbeef
Copy link
Owner

I have a solution for this (or at least @wata727's issue) but it breaks a lot of warning fingerprints. I'll add it eventually but it may be a little bit longer.

Repository owner locked and limited conversation to collaborators Jul 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants