Segfault when calling PG::Result#result_error_message after PG::Result#clear

Issue #185 resolved
Former user created an issue

Knowing the C API, it's clear why this happens. To prevent this, you probably want to have methods such as #result_error_message raise an exception if DATA_PTR(self) == NULL .

Comments (15)

  1. Lars Kanis

    Could you please send some more information like stacktrace or sample code? Calling PG::Result#result_error_message after PG::Result#clear gives a PG::Error: result has been cleared for me. The check you mentioned is already in.

    It would also be nice to sign up to bitbucket instead of posting anonymous.

  2. Jeremy Evans

    Sorry for the lack of detail in the previous post. I also took the time to sign up.

    You are right in the general case that you get a "PG::Error: result has been cleared" exception. However, this isn't true when using set_notice_receiver. Here's the example code:

    c = PGconn.connect({})
    a = nil
    c.set_notice_receiver{|r| a = r}
    res = c.async_exec("DO 'BEGIN\nRAISE WARNING ''foo'';\nEND;'")

    You can also get a abort trap by calling #clear on the PG::Result passed to the notice receiver block:

    c = PGconn.connect({})
    c.set_notice_receiver{|r| r.clear}
    res = c.async_exec("DO 'BEGIN\nRAISE WARNING ''foo'';\nEND;'")

    Here's a gdb backtrace of the first error:

    #0  0x02ff8fcd in kill () at <stdin>:2
    #1  0x03063a16 in raise (s=6) at /usr/src/lib/libc/gen/raise.c:39
    #2  0x03063943 in abort () at /usr/src/lib/libc/stdlib/abort.c:70
    #3  0x0ac2238d in rb_bug () from /usr/local/lib/
    #4  0x0ace0f44 in sigsegv () from /usr/local/lib/
    #5  <signal handler called>
    #6  strlen (str=0xdfdfdfdf <Address 0xdfdfdfdf out of bounds>) at /usr/src/lib/libc/string/strlen.c:43
    #7  0x0acf1bf6 in rb_str_new_cstr () from /usr/local/lib/
    #8  0x0acf1dbd in rb_tainted_str_new_cstr () from /usr/local/lib/
    #9  0x08ab1a54 in pgresult_error_message () from /usr/local/lib/ruby/gems/2.1/gems/pg-0.17.0/lib/
    #10 0x0ad3a5cf in call_cfunc_0 () from /usr/local/lib/
    #11 0x0ad40dd3 in vm_call_cfunc () from /usr/local/lib/
    #12 0x0ad51cce in vm_call_method () from /usr/local/lib/
    #13 0x0ad499c8 in vm_exec_core () from /usr/local/lib/
    #14 0x0ad4f069 in vm_exec () from /usr/local/lib/

    I stumbled upon this issue while writing specs for Sequel's support for set_notice_receiver.

  3. Lars Kanis

    Thank you Jeremy! I thought about this issue two years ago and added a note to the documentation: But yes, your solution is definitely better!

    I think the ensure part is not strictly necessary, since raising an exception and therefore longjmp'ing over the libpq callback return code path is probably no supported libpq use case and will not call PQclear anyway. Nevertheless it's safe to do it with ensure.

  4. Lars Kanis

    This issue is fixed on the default branch.

    @Jeremy Evans, unfortunately it probably also breaks your sequel_pg extension. Maybe you can update it, so that it makes use of ruby-pg's type cast extension and streaming support. We also could add some more value retrieval methods to PG::Result, to better support sequel. In the end we could remove all C code from sequel_pg by moving it to ruby-pg or replace it with code already in ruby-pg.

  5. Jeremy Evans

    @Lars Kanis Thanks for the heads up about pg 0.18+ breaking sequel_pg. Unfortunately, I don't have time now to update sequel_pg to use the new type casting features in ruby-pg, but I did add a quick patch so that sequel_pg will work with both the old and new versions of ruby-pg. To get the underlying pointers to PGconn and PGresult, I'm calling C functions defined in ruby-pg, so as long as you don't mark the functions static, this should work fine. Anyway, let me know what you think:

  6. Jeremy Evans

    @Lars Kanis I noticed that when I tried to build a Windows binary gem. I don't consider it a big loss. sequel_pg is purely a performance enhancing gem, and if you are running ruby on Windows, you probably don't care about performance.

  7. Log in to comment