Gurusamy Sarathy wrote: > Thanks. I have a few questions. > > >! SV *sv = get_sv("DynaLoader::dl_debug", 0); > >--- 28,64 ---- > >! SV *sv = perl_get_sv("DynaLoader::dl_debug", 0); > > Why was this needed? (The preferred name is get_sv(), the long form of > which is Perl_get_sv(aTHX_ ...).) Not according to perlguts it isn't.... | If you know the name of a scalar variable, you can get a pointer to its SV | by using the following: | | SV* perl_get_sv("package::varname", FALSE); > >+ if ((sub = perl_get_cv("DynaLoader::dl_unload_file", FALSE)) != NULL) { > >+ dl_librefs = perl_get_av("DynaLoader::dl_librefs", FALSE); > >+ while ((dl_libref = av_pop(dl_librefs)) != &PL_sv_undef) { > >+ dSP; > >+ ENTER; > >+ SAVETMPS; > >+ PUSHMARK(SP); > >+ XPUSHs(sv_2mortal(dl_libref)); > >+ PUTBACK; > >+ perl_call_sv(sub, G_DISCARD); > >+ FREETMPS; > >+ LEAVE; > > I'd s/perl_// those, purely as a matter of style. Also, not according to perlguts. I've never written a core patch before, so I had to use the only documentation to hand, which is perlguts. Thank yourself lucky that I figured out the item popped off @dl_librefs had to be mortalised ;-) (perlguts virtually always fails to tell you if a return value is mortal or not). > And dl_unload_all_files() calling into Perl again via an XSUB instead of > calling a C function directly seems suboptimal. Perhaps the list of > dlopen()ed files should be maintained in the C code, so that you won't > have to depend on the Perl symbol table being around. Optimal, no. Orthogonal, yes. All the other dl_XXX functions are declared in the sundry platform-dependent dl_XXX.xs files. the Principal Of Least Surprise suggested that dl_unload_file() should be made to look as similar to its partner dl_load_file() as possible. Also, doing this meant that I could check to see if dl_unload_file() was declared before I tried to call it - meaning that on those platforms that didn't require it, dl_unload_file() could safely be left undefined. You couldn't do this if it was written as a plain C function - I'd have had to put a stub dl_unload_file() in every dl_XXX.xs platform variant. As for efficiency - the overead of calling dlclose() will dwarf the overhead of calling an XSUB. As for depending on the Perl symbol table being around - when this patch was originally discussed this was raised, and I got the distinct impression that the reason for rejecting my previous patch and suggesting the use of Perl_call_atexit() was precisely because the symbol table *would* be around. > > #endif > >+ Perl_call_atexit(&dl_unload_all_files, (void*)0); > > } > > s/Perl_// here, or it won't work with multiplicity/usethreads builds. Perl_call_atexit/call_atexit aren't documented anywhere I could find, and the only use is in perlapi.c, where it is called as '((CPerlObj*)pPerl)->Perl_call_atexit(fn, ptr);' so is that broke too? > There was some doubt in my mind whether call_atexit() would do the job > properly. What happens if the dll has allocated SVs? The call_atexit() > callbacks are called after objects are destroyed but *before* the SV > arenas are deallocated. Do you foresee any problems from that? Pass. Does that mean that @dl_librefs will have been reclaimed? If so, the patch seems to work remarkably well considering it is popping stuff of a free'd AV. I don't think the dll allocating SVs has anything to do with it - dlclose() doesn't do any sort of cleanup (except if you have a .fini section, but that is probably Solaris-specific). I wrote the patch based on what I thought was the consensus on the correct way to do this - was I mislead? Alan BurlisonThread Previous | Thread Next