On Mon, Mar 03, 2003 at 10:16:07AM +1100, Stas Bekman wrote:
> Tim Bunce wrote:
> >On Sun, Mar 02, 2003 at 02:21:41PM +1100, Stas Bekman wrote:
> >>>
> >>>Seems like the $dbh->take_imp_data method will need driver involvement
> >>>to 'cleanse' the imp data by freeing everything not essential to
> >>>the connection - and hopefully that'll include all the SVs.
> >>>
> >>>If SVs are required, for some reason, can they be made shared, or
> >>>replaced with a shared copy? (My perl threads api knowledge is a
> >>>bit rusty.)
> >>
> >>There should be no perl datastructures involved at all in the pool
> >>storage.
> >
> >Maybe not 'involved' but there's a strong chance the imp_dbh contains
> >SV*'s.
>
> I thought that drivers are pure C.
DBD::Foo's are a hybrid interfacing a pure C database API to a perl application.
But I've checked just checked a whole bunch of DBD's now...
Most do contain SV*'s in the imp_sth struct, but the imp_dbh doesn't
have any, so it almost seems like this isn't going to be a big issue
in practice.
There are some exceptions though... DBD::ODBC does have
SV *odbc_err_handler; /* contains the error handler coderef */
but I think that could be moved into the $dbh attribute hash
in a new release. ["Paging Mr Urlwin"]
http://search.cpan.org/src/JURL/DBD-ODBC-1.04/dbdimp.h
And DBD::Sybase has the same, plus a row callback hook SV:
http://search.cpan.org/src/MEWP/DBD-Sybase-0.95/dbdimp.h
["Paging Mr. Peppler"]
> >If imp_dbh contains an SV* in the drivers portion and the imp_dbh
> >was allocated by thread A. Thread A deposits it into the pool
> >(copying the imp_dbh and the SV pointer) and then exits. Another thread
> >gets a copy of the imp_dbh from the pool, the DBI wraps a handle around it
> >and the driver reinitialises the drivers portion. In doing that it will
> >try to copy the contents of the SV*. That's now very dangerous as the
> >thread that allocated the original one has not exited.
>
> s/not exited/exited/
[Ooops, yes.]
> That's why I've suggested that this works cleanly if the driver's imp
> portion includes no SV*s. In case it does it's up to the driver to figure
> out how to give away a mirror of its private imp portion, including SV*s if
> any. In this case DBI by itself can't just memcpy the imp_dbh_st struct,
> but it has to perform a deep copy on SV*s (without affecting the reference
> count of the original SV*s). Depending on what kind of SV*s are stored in
> that private portion, another thread may or may not revive them. After all
> SV*s are just datastruct, so created by one interpreter and copied away as
> raw data, they can be revived by another interpreter, though that other
> interpreter won't destroy this data, since it has no clue it belongs to it.
>
> So, I believe it's up to the driver to decide whether its data can be
> cloned and pooled or not. I believe some chunks of perl_clone() may help
> here.
Umm, I'm not sure if they'll be any issues about which thread does the cloning.
But I suspect that deep cloning would rarely be what's needed.
> So far I've played with DBD::mysql, which has no SV*s in its private imp
> and it works great.
It looks like others will require little or no change either.
Some, like DBD::Interbase may have some other issues though.
It does it's own explicit housekeeping of statement handles
by keeping them in a list rooted in the imp_dbh:
http://search.cpan.org/src/EDPRATOMO/DBD-InterBase-0.40/dbdimp.h
Here again, I think this translates to the driver implementing the
take_imp_data() method and it either dealing with all the cleanup
issues or croaking if it can't. The driver needs to view the
take_imp_data method as being nearly the same as disconnect+DESTROY
only not actually calling the database API to disconnect.
All SVs and other state can be cleaned up and feeded - in the case
of Interbase that probably translates into forcing destruction of
any statement handles that still exist when take_imp_data() is called.
All that needs to remain is the underlying database API conection data.
> >Actually, it ought to have been:
> >
> > struct imp_dbh_st {
> > dbih_dbc_t *com; /* POINTER to com MUST be first */
> > imp_t imp;
> > }
> >
> >which would then make the drivers independant of the changes to the
> >size of the com structure.
>
> I guess drivers could already do that now, so in the future they don't have
> to do anything to adher to the change.
The DBI would have to do it as it supplies a whole bunch of macros
that access the com element.
> >It's on my list of things to do in the future. But I'd like to do
> >it as part of a general overhaul of the DBI/DBD interface and flag
> >the results a DBI 2.00. But I'm not sure when that'll happen.
The issue being that changing the DBI/DBD interface forces a site
to recompile all their drivers, which makes installing that release
a "big deal". Hence my desire to not make changes there very often,
and when I do to make several together.
Tim.
