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

Array Decoder Leaks Memory if Scalar Decoder Raises #279

Closed
ged opened this issue Jun 24, 2018 · 2 comments
Closed

Array Decoder Leaks Memory if Scalar Decoder Raises #279

ged opened this issue Jun 24, 2018 · 2 comments

Comments

@ged
Copy link
Owner

ged commented Jun 24, 2018

Original report by Jeremy Evans (Bitbucket: jeremyevans, GitHub: jeremyevans).


pg uses xmalloc/free for the temporary buffer for array elements, without using rb_ensure to ensure that free is called if an exception is raised by decoding. This means if a decoder raises an error, the memory is leaked. Example test case:

#!ruby

require 'pg'
conn = PG.connect
class ByteaLeaker < PG::SimpleDecoder
  def decode(string, tuple=nil, field=nil)
    raise
  end
end
PG::BasicTypeRegistry.register_type(0, 'bytea', nil, ByteaLeaker)
map = conn.type_map_for_results = PG::BasicTypeMapForResults.new(conn)
GC.start
system("ps aux -p #{$$}")
100.times do
  print '.'
  res = nil
  begin
    res = conn.exec("SELECT ARRAY['#{'a'*1000000}']::bytea[]")
    res.getvalue(0,0)
  rescue RuntimeError
  ensure
    res.clear if res
  end
end
GC.start
system("ps aux -p #{$$}")

FYI, sequel_pg uses rb_str_buf_new for the temporary buffer to avoid needing rb_ensure, but make sure you use RB_GC_GUARD appropriately if you do that.

There is a similar leak in pg_text_dec_bytea, where the buffer returned by PQunescapeBytea is leaked if rb_tainted_str_new fails. That's harder to hit, but if you are out of memory, leaking memory is going to make things worse. sequel_pg now uses rb_ensure in that case to avoid leaking memory.

@ged
Copy link
Owner Author

ged commented Jun 25, 2018

Original comment by Lars Kanis (Bitbucket: larskanis, GitHub: larskanis).


Good catch! I usually tend to use strings or data objects for memory allocations instead of rb_ensure() . But I'll check this! Thanks!

@ged
Copy link
Owner Author

ged commented Jul 29, 2018

Original comment by Lars Kanis (Bitbucket: larskanis, GitHub: larskanis).


Array-Decoder: Avoid leaking memory when an Exception is raised while parsing

This can happen with malformed arrays or within elements decoders.

Fixes #279

@ged ged closed this as completed Jul 29, 2018
@ged ged added this to the Pending milestone Oct 8, 2019
jeremyevans pushed a commit to jeremyevans/ruby-pg that referenced this issue Jan 2, 2020
.. even if rb_tainted_str_new() raises an Exception.

Related to issue ged#279.
jeremyevans pushed a commit to jeremyevans/ruby-pg that referenced this issue Jan 2, 2020
… parsing

This can happen with malformed arrays or within elements decoders.

Fixes ged#279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant