Comment 10 for bug 1133093

Revision history for this message
Antony T Curtis (atcurtis) wrote : Re: [Bug 1133093] Re: Crash on ORDER BY in OQGRAPH v3

How it was was probably correct - intrusive_ptr can be confusing - what you may need to look at is constructor initialisation or use in-place initialisation elsewhere otherwise, the explicit in-place you're doing will leave memory leak.

On 15 Aug, 2013, at 7:23 am, Andrew McDonnell wrote:

> Summary of status:
>
> I think we can close this and open a new one against the debug build.
> Maybe.
>
> The raw segfault was caused by something using a thing called an
> 'intrusive_ptr' which is used to add reference counting. However its
> copy cnstructor has odd sematics such that the assignment operator
> actually does a swap operation with the assignee(!), which means if the
> memory you are assigning to is uninitialised, you end up with junk in
> where you were copying from, which causes a crash when the assignee is
> destructed.
>
> #3 <signal handler called>
> #4 0x000000000098f292 in oqgraph3::intrusive_ptr_release (ptr=0xa5a5a5a5a5a5a5a5) at /home/andrew/develop/maria/repo/oqgraph-varchar/storage/oqgraph/oqgraph_thunk.h:132
> #5 0x0000000000991e19 in boost::intrusive_ptr<oqgraph3::cursor>::~intrusive_ptr (this=0x7fe72e1f5ed0, __in_chrg=<optimized out>) at /usr/include/boost/smart_ptr/intrusive_ptr.hpp:101
> #6 0x0000000000991f1a in boost::intrusive_ptr<oqgraph3::cursor>::operator= (this=0x22bef68, rhs=...) at /usr/include/boost/smart_ptr/intrusive_ptr.hpp:133
> #7 0x000000000098f1a5 in oqgraph3::cursor_ptr::operator= (this=0x22bef68) at /home/andrew/develop/maria/repo/oqgraph-varchar/storage/oqgraph/oqgraph_thunk.h:58
>
> The memory coming from the SQL core (the 'ref') via index_read is
> uninitalised when we first see it, but we seem to use it as a persistent
> storage area (I dont fully understand how all thatworks as yet, and
> ha_example.cc "explains" it but doesnt really...)
>
> Thus, in-place should theoretically sort that one out:
>
> if (cursor)
> cursor->current(ref);
> else
> - ref= reference();
> + new (ref_ptr) reference();
>
> This actually passes the regression test now in a normal build!
> In a debug build however, it is failing... tracing through with the debugger I worked out we can reset the contents of the ref via ha_oqgraph::index_read which seems to be called once at the start of the query:
>
> {
> DBUG_ASSERT(inited==INDEX);
> + graph->row_ref((void*) ref); // reset before we have a cursor, so the memory is inited, avoiding the sefgault in position() when select with order by (bug #1133093)
> return index_read_idx(buf, active_index, key, key_len, find_flag);
> }
>
> THIS now fixes the segfault, but causes an assertion indicating we
> havent properly closed something in my_isam; specifically, the
> edges->file table, we have missed a closing ha_index_end(). This is
> where I finally got stuck...
>
> The 'normal' build still passes...
>
> --
> You received this bug notification because you are a member of Open
> Query core, which is subscribed to OQGRAPH.
> https://bugs.launchpad.net/bugs/1133093
>
> Title:
> Crash on ORDER BY in OQGRAPH v3
>
> Status in OQGraph Engine for MariaDB:
> Triaged
>
> Bug description:
> SELECT * FROM graph WHERE latch=1 AND origid=1 AND destid=6;
> works
>
> SELECT * FROM graph WHERE latch=1 AND origid=1 AND destid=6 ORDER BY seq;
> segfaults.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/oqgraph/+bug/1133093/+subscriptions