On Sun, Mar 02, 2003 at 02:21:41PM +1100, Stas Bekman wrote:
Tim Bunce wrote: [...]
Now, back to memory management...
As I've mentioned recently, I figure the 'thing' that's placed into the pool is a scalar string that holds a copy of the entire imp data structure. Doing that, rather than passing an integer pointer around, means we avoid issues over which interpreter owns and frees that memory.
That just leaves two other related issues... a. How database API data gets cleaned up. b. Who (which interpreter) owns and frees any SVs _within_ the imp data.
The best way to handle (a) would be to create a new dbh using the imp data and then let the dbh get destroyed 'normally' (not pass the imp data back to the pool).
For (b) we may be able to duck the issue to a certain extent because all we're really trying to pass around is the pointers from the database API. There should be no need to use any SV pointers. On the other hand, any SVs need to get freed at some point. Only the driver know where they are so the driver has to be involved.
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.
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/
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.
So far I've played with DBD::mysql, which has no SV*s in its private imp and it works great.
Finally the only fishy thing is the following: Since we memcpy the whole imp_dbh, including the com struct, the memory used by that struct (first n bytes) is invalid after the original dbh has been destroyed and should never be accessed. In fact we may want to null it all.
I'd expect the take_imp_data method to do that and clear the IMP_COM_SET flag.
Good.
In retrospect I wanted to ask why DBI tells driver to define struct imp_xxh_st as:
struct imp_dbh_st { dbih_dbc_t com; /* MUST be first element in structure */ private element1; ... private elementN; }
and not:
typedef struct private_data imp_t; struct imp_dbh_st { dbih_dbc_t com; /* MUST be first element in structure */ imp_t imp; /* MUST be the second element and the only one */ }
where imp_t could have any number of elements, while staying opaque to the DBI frontend.
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.
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.
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
