← Back to team overview

oqgraph-dev team mailing list archive

Re: Fwd: Re: Serious use after free bug

 

Thanks Antony

Just to recap, currently:

* The backing table is opened using open_table_from_share() in ha_oqgraph::open()
* open_table_from_share() takes a THD* as an argument, this is what eventually
goes away.
* An instance of open_query::oqgraph_share is created as well, this is what
holds onto the TABLE that has been associated with the THD
* The TABLE is also retained in the ha_oqgraph instances as field edges
* The TABLE is eventually closed in ha_oqgraph::close()

So it seems ha_oqgraph::open() does two things:

* validates the table,
* create an open_query::oqgraph_share instance

There is another method, ha_oqgraph::external_lock(), where we release a
cursor then fall through to the backing table oqgraph::external_lock().

Perhaps this is the place we should then let edges go? And then some clear it
from the graph_share? And then we need to work out how to get another one back
on the next connection?

--Andrew





On 19/06/14 01:04, Antony T Curtis wrote:
> It used to be possible to know if a storage engine handler is in use because of the older lock semantics.
> 
> So, upon unlock, one should release any table dependency and upon lock, acquire it.
> 
> However, I think this may be changing in newer MySQL/MariaDB so one may have to consider alternative mechanism, such as pretending that the table is transactional and then releasing dependant table upon commit/rollback.
> 
> In any case, oqgraph probably should not be holding on to any TABLE reference while in the table cache.
> 
> I made some edits in a local tree for when I was playing with oqgraph on toku… I must check to see if all changes made it upstream.
> 
> Regards,
> Antony.
> 
> On Jun 18, 2014, at 4:59 AM, Andrew McDonnell <bugs@xxxxxxxxxxxxxxxxxxx> wrote:
> 
>> Sure thing, I'll look at the others as well.  I meant to say federatedx BTW
>> not federated, I saw the readme. Also I meant reset() not disconnect()! (oops)
>>
>> On 18/06/14 18:39, Arjen Lentz wrote:
>>> Oops I intended to send to list.
>>>
>>>
>>> Hi Andrew
>>>
>>>> Arjen Lentz <arjen@xxxxxxxxxxxxxxxx> wrote:
>>>>> You may need some advice on how to ensure that when a connection closes,
>>>>> things get cleaned up properly. We may have missed a hook of some sort.
>>>>> You could also check this in another engine. Perhaps Federated, or
>>>>> Spider, or CONNECT ?
>>>
>>> On 18/06/14 19:03, Andrew McDonnell wrote:
>>>> From a very cursory look at federated it seems we dont have
>>>> `handler::disconnect()` so I'll probably start there.
>>>> Meantime in the medium term I think refactoring ha_oqgraph.cc
>>>> to look more like ha_federated would be a worthwhile activity
>>>
>>> Federated is probably not the best example.
>>> It's in the best case a fixed up version of something quite old.
>>> Look at others also (CONNECT is the newest, but Spider also - and
>>> perhaps TokuDB ?) and roughly document which we can then run by Serg and
>>> others first.
>>> This also provides the opportunity to get other people in the community
>>> involved in some of the refactoring work, which has the advantage of
>>> getting more people familiar with the codebase!
>>>
>>>
>>> Regards,
>>> Arjen.
>>
>>
>> -- 
>> Mailing list: https://launchpad.net/~oqgraph-dev
>> Post to     : oqgraph-dev@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~oqgraph-dev
>> More help   : https://help.launchpad.net/ListHelp
> 


References