On Sun, Feb 05, 2012 at 09:04:23PM +0000, Dave Mitchell wrote:
> On Mon, Jan 30, 2012 at 10:56:37PM +0000, Tim Bunce wrote:
> >
> > dbih_getcom2 does the first part of that when looking up DBI hande
> > magic. I never added the "move to the top" because I figured it would be
> > very rare for magic to get added to the inner hash of a DBI handle.
> > It seems even less likely that someone would add magic to a DBI method,
> > but you never know. Unless you can think of a plausible case I'd be
> > happy if you just added the short-cut+fallback and skipped the shuffle.
>
> I'd already written the magic shuffler code by the time I saw your email,
> so unless you want me to rip it out, it's your's fro free!
:)
> Anyway, attached is the final patch.
> I've tested it under 5.8.1, 5.8.9, 5.14.2 and 5.15.7, with various
> permutations of threaded builds.
>
> It took quite a bit more work from the initial draft I showed you, mainly
> getting the threaded stuff right, and a stupid PL_generation /
> PL_sub_generation mix-up. I knew what the difference was between the two
> vars, but whenever I visually inspected the code, I kept seeing the former
> as the latter and couldn't understand why cache invalidation didn't work!
> - imp_msv = (SV*)gv_fetchmethod_autoload(DBIc_IMP_STASH(imp_xxh),
> meth_name, FALSE);
> + if (is_orig_method_name)
> + imp_msv = (SV*)inner_method_lookup(aTHX_ DBIc_IMP_STASH(imp_xxh),
> + cv, meth_name);
> + else
> + imp_msv = (SV*)gv_fetchmethod_autoload(DBIc_IMP_STASH(imp_xxh),
> + meth_name, FALSE);
>
> /* if method was a 'func' then try falling back to real 'func'
> method */
> if (!imp_msv && (ima_flags & IMA_FUNC_REDIRECT)) {
> @@ -3463,7 +3582,7 @@
>
> - if (!imp_msv) {
> + if (!imp_msv || !GvCV(imp_msv)) {
What's the GvCV test for?
> +++ t/31methcache.t (revision 0)
The tests look great. Thanks.
> +BEGIN { eval "use threads;" } # Must be first
> +my $use_threads_err = $@;
> +# With this test code and threads, 5.8.1 has issues with freeing freed
> +# scalars, while 5.8.9 doesn't; I don't know about in-between - DAPM
"issues"? i.e., warnings at global destruction?
The Mac OS X perl 5.8.8 with threads that I have handy works ok.
> +my $has_threads = $Config{useithreads} && $] >= 5.008009;
> +die $use_threads_err if $has_threads && $use_threads_err;
Umm.
I'm tempted to disable the cache at compile time for perl < 5.8.8.
Meanwhile, I've applied the patch and uploaded a trial release
for cpantesters to chew on. Once there are sufficient reports from old
perls that have threading enabled I'll scan them for warnings.
Thanks again Dave!
Tim.
