Re: apr_dso_handle_close ?
Hell, if you have your own pool, and you need to unload 30 modules, you're only going to consume 480 bytes. Tops. Effectively, there is no allocation overhead, so that isn't a valid reason to start moving outside of the standard apr_FOO_t design of APR. For a workaround because Perl doesn't give you a function. and what about the mutex lock on alloc_mutex? and all the extra function calls i mentioned before: also with apr_dso_make there'd also 6 * num_dsos_to_close extra function calls, apr_dso_make, apr_pcalloc, apr_pool_cleanup_register, apr_dso_unload, apr_pool_cleanup_run, apr_pool_cleanup_kill. not to mention having to loop over p-cleanups * num_dsos_to_close in apr_pool_cleanup_kill. sure it might be minmal overhead, but it is overhead, period. a little here, a little there, this stuff adds up. but if the exposure of apr_dso_handle_close() isn't acceptable because it doesn't feel right, i guess apr_dso_make() or whatever will do the trick until a Perl is released with the proper interface. I would like David Reid and Brian Havard (and maybe Fred Sanchez and either Bill)to sign off on the change before we make it. I'm being paranoid about those platforms that I don't know really well, and I know I am, but I really don't want to be wrong about this change. As long as those guys are comfortable with it, then +1 for making the change. Of course, since it could be a long time before they all respond, I would simply ask that we wait a day or two longer, and the commit message makes it clear that this change can be removed if it causes any problems on some platforms. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: apr_dso_handle_close ?
On Tuesday, April 17, 2001, at 08:38 PM, [EMAIL PROTECTED] wrote: I would like David Reid and Brian Havard (and maybe Fred Sanchez and either Bill)to sign off on the change before we make it. I don't think that this will make the DSO code for dyld harder to write... If you give me a day or two, I can verify this by implementing DSO for dyld. But I don't object on those grounds. However, I do find it bothersome to write API that close a void* handle on a object that we didn't necessarily create; this strikes me as a hack. If we didn't apr_dso_open this handle, it makes no sense to me that we should apr_dso_close it. I don't consider merely exposing private functionality to be any sort of justification. One we expose API, then we're stuck with it. So either mod_perl has to suffer though this until Perl 5.6.2 or APR has to support this forever. I lean towards the former. But I'm not an APR architect, and so I defer to Ryan/Greg/etc. as to whether such exposure is correct and acceptable. -Fred
Re: apr_dso_handle_close ?
From: [EMAIL PROTECTED] Sent: Tuesday, April 17, 2001 10:38 PM Hell, if you have your own pool, and you need to unload 30 modules, you're only going to consume 480 bytes. Tops. Effectively, there is no allocation overhead, so that isn't a valid reason to start moving outside of the standard apr_FOO_t design of APR. For a workaround because Perl doesn't give you a function. and what about the mutex lock on alloc_mutex? and all the extra function calls i mentioned before: also with apr_dso_make there'd also 6 * num_dsos_to_close extra function calls, apr_dso_make, apr_pcalloc, apr_pool_cleanup_register, apr_dso_unload, apr_pool_cleanup_run, apr_pool_cleanup_kill. not to mention having to loop over p-cleanups * num_dsos_to_close in apr_pool_cleanup_kill. sure it might be minmal overhead, but it is overhead, period. a little here, a little there, this stuff adds up. If we were discussing operation, then I'd agree. We are talking about startup shutdown behavior, so I'm in favor of Greg's position here, stay with the interface we've defined for other such beasts. but if the exposure of apr_dso_handle_close() isn't acceptable because it doesn't feel right, i guess apr_dso_make() or whatever will do the trick until a Perl is released with the proper interface. We can all accept an os - dso mechanism. But it's not APR's job to make 'other' mechanisms platform-independent, it's a complete wrapper to provide such services. Patching Perl is the way to go, but ITMT, if you can verify (through sizeof() or whatever) that we have an OS handle, then go for it. There will just need to be some mod_perl mechanism to assure that the perl module* == apr's dso os module*. I would like David Reid and Brian Havard (and maybe Fred Sanchez and either Bill)to sign off on the change before we make it. I'm being paranoid about those platforms that I don't know really well, and I know I am, but I really don't want to be wrong about this change. We can all implement it, dreid and trawick are the folks on truly 'funky' platforms. The MPE guys might have some obscure issues, as well. If mod_perl marks those as incompatible, we should be good. As long as those guys are comfortable with it, then +1 for making the change. Of course, since it could be a long time before they all respond, I would simply ask that we wait a day or two longer, and the commit message makes it clear that this change can be removed if it causes any problems on some platforms. What are you suggesting? We have a simple solution, APR_ENOTIMPL. If the platform can't handle a 'simple' native dso*, then it better return APR_ENOTIMPL. That isn't the same as testing that the os's dso mechanism we use is the same as perl's. It will have to be up to mod_perl to make that distinction. Bill
Re: apr_dso_handle_close ?
On Tue, 17 Apr 2001 20:38:42 -0700 (PDT), [EMAIL PROTECTED] wrote: Hell, if you have your own pool, and you need to unload 30 modules, you're only going to consume 480 bytes. Tops. Effectively, there is no allocation overhead, so that isn't a valid reason to start moving outside of the standard apr_FOO_t design of APR. For a workaround because Perl doesn't give you a function. and what about the mutex lock on alloc_mutex? and all the extra function calls i mentioned before: also with apr_dso_make there'd also 6 * num_dsos_to_close extra function calls, apr_dso_make, apr_pcalloc, apr_pool_cleanup_register, apr_dso_unload, apr_pool_cleanup_run, apr_pool_cleanup_kill. not to mention having to loop over p-cleanups * num_dsos_to_close in apr_pool_cleanup_kill. sure it might be minmal overhead, but it is overhead, period. a little here, a little there, this stuff adds up. but if the exposure of apr_dso_handle_close() isn't acceptable because it doesn't feel right, i guess apr_dso_make() or whatever will do the trick until a Perl is released with the proper interface. I would like David Reid and Brian Havard (and maybe Fred Sanchez and either Bill)to sign off on the change before we make it. I'm being paranoid about those platforms that I don't know really well, and I know I am, but I really don't want to be wrong about this change. As long as those guys are comfortable with it, then +1 for making the change. Of course, since it could be a long time before they all respond, I would simply ask that we wait a day or two longer, and the commit message makes it clear that this change can be removed if it causes any problems on some platforms. Well, I'm not all that keen on the concept but the patch looks like it will work. I haven't actually tested it but it compiles, issuing a few warnings as the native handle type is an unsigned long, not trivially convertable to/from a void * dso.c: In function `apr_dso_handle_close': dso.c:64: warning: passing arg 1 of `DosFreeModule' makes integer from pointer without a cast dso.c: In function `dso_cleanup': dso.c:76: warning: passing arg 1 of `apr_dso_handle_close' makes pointer from integer without a cast If we must have such a function it should take an apr_os_dso_t type arg, not a void *. -- __ | Brian Havard | He is not the messiah! | | [EMAIL PROTECTED] | He's a very naughty boy! - Life of Brian | --
Re: apr_dso_handle_close ?
Greg Stein [EMAIL PROTECTED] writes: I'm sorry, but this is just feeling wrong. Perl opens these DSOs for me, but won't close them. I can't patch Perl to do it for me... lessee... oh! I can patch APR! Yah, that's it! I'm not comfortable with APR being a workaround for other problems. APR has a design: you build an apr_FOO_t and use that. We do have a notion of getting/setting a native FOO into those structures. yep... it is unfortunate that Perl doesn't have a way to close them but it is easy enough to solve outside of APR if the existing APR way to unnaturally (i.e., via apr_os_put|make_foo) is not deemed elegant enough -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: apr_dso_handle_close ?
Doug MacEachern [EMAIL PROTECTED] writes: below is a complete patch to implement apr_dso_handle_close() the only cleanup that saved into apr_dso_handle_t was os390: dso-failing_errno = errno; which is pointless since that value is returned by the function and nothing was checking it anyways. the reason it is saved is because of the APR interface for getting an error string from a dso load error... you don't pass in the system-specific error value (e.g., errno on Unix) but instead you pass in the apr DSO handle... since strerror() is allowed to overlay the buffer containing the error string, we don't simply save the strerror() result but instead save the errno... as for why the APR interface is designed that way: some platforms have additional information available (beyond any sort of errno-like numeric value)... -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: apr_dso_handle_close ?
i don't want to push apr_dso_handle_close() anymore, i understand that it is not right for the public apr api. consider my patch recinded. i will submit a patch for apr_dso_make, if i can cvs up my tree (its been hanging all morning). and will patch Perl to make the native close available from there.
Re: apr_dso_make??
FWIW, I've just committed the start of the apr_os_dso_ support as it was
missing and we probably should have had it in a while back. No code to back
up the headers and defines but I'll get some done on the plane to Denver
tomorrow. If I can get a network connection somewhere then I'll be able to
commit, but if I'm stuck with dial up then it'll ahve to wait until I get
home.
david
- Original Message -
From: David Reid [EMAIL PROTECTED]
To: APR Development List dev@apr.apache.org
Sent: Wednesday, April 18, 2001 7:51 PM
Subject: Re: apr_dso_make??
Forgive my poor eyesight, but are we actually defining an apr_os_dso_t
anywhere?
A snippit from apr_portable.h
#elif defined(__BEOS__)
#include kernel/OS.h
struct apr_os_lock_t {
/* Inter proc */
sem_id sem_interproc;
int32 ben_interproc;
/* Intra Proc */
sem_id sem_intraproc;
int32 ben_intraproc;
};
typedef int apr_os_file_t;
typedef DIR apr_os_dir_t;
typedef int apr_os_sock_t;
typedef struct apr_os_lock_t apr_os_lock_t;
typedef thread_id apr_os_thread_t;
typedef thread_id apr_os_proc_t;
typedef int apr_os_threadkey_t;
typedef struct timevalapr_os_imp_time_t;
typedef struct tm apr_os_exp_time_t;
#else
/* Any other OS should go above this one. This is the lowest common
* denominator typedefs for all UNIX-like systems. :)
*/
#ifdef NEED_UNION_SEMUN
/* it makes no sense, but this isn't defined on solaris */
union semun {
long val;
struct semid_ds *buf;
ushort *array;
};
#endif
struct apr_os_lock_t {
#if APR_USE_SYSVSEM_SERIALIZE
int crossproc;
#elif APR_USE_FCNTL_SERIALIZE
int crossproc;
#elif APR_USE_PROC_PTHREAD_SERIALIZE
pthread_mutex_t *crossproc;
#elif APR_USE_FLOCK_SERIALIZE
int crossproc;
#else
/* No Interprocess serialization, too bad. */
#endif
#if APR_HAS_THREADS
/* If no threads, no need for thread locks */
#if APR_USE_PTHREAD_SERIALIZE
pthread_mutex_t *intraproc;
#endif
#endif
};
typedef int apr_os_file_t;
typedef DIR apr_os_dir_t;
typedef int apr_os_sock_t;
typedef struct apr_os_lock_t apr_os_lock_t;
#if APR_HAS_THREADS APR_HAVE_PTHREAD_H
typedef pthread_t apr_os_thread_t;
typedef pthread_key_t apr_os_threadkey_t;
#endif
typedef pid_t apr_os_proc_t;
typedef struct timevalapr_os_imp_time_t;
typedef struct tm apr_os_exp_time_t;
#endif
Now I don't see that we've actually talked about apr_os_dso_t's at any
point
in this file and I'm not sure it's even been added to APR and as such
that's
probably what we should be doing! Then we simply follow the API we
already
have don't we?
Sorry if I'm missing something here but I've had a touch of sunstroke!!
david
(copied to doug first and then to the list as reply-all got me again!) :)
- Original Message -
From: Doug MacEachern [EMAIL PROTECTED]
To: David Reid [EMAIL PROTECTED]
Cc: APR Development List dev@apr.apache.org
Sent: Wednesday, April 18, 2001 7:40 PM
Subject: Re: apr_dso_make??
On Wed, 18 Apr 2001, David Reid wrote:
Remind me again, why aren't we just going to add apr_dso_os_put (or
whatever
combination of the bits in the correct order is these days) and the
_get
version to use apr_os_dso_t's? That *would* keep us in line with the
rest
of the APR API wouldn't it?
since apr_dso_handle_t is an incomplete type, we need to `make' an
apr_dso_handle_t:
apr_dso_handle_t *dso;
status = apr_dso_make(p, dso, (void *)native_handle); /* will alloc
*dso
*/
i suppose it should be:
status = apr_dso_make(p, dso);
apr_os_dso_handle_put(dso, (void *)native_handle);
and perhaps:
void *os_handle = apr_os_dso_handle_get(dso);
Clean up of DSO autoconfizification
I don't like how we detect whether DSO support should be enabled by
testing for the possible implementation methods, and then rather than
remembering which we found, we have all this #if THIS_OS crap all over
dso/unix/dso.c. If we found dlfcn or shl or whatever, that should be
what we conditionalize on, and maybe have per-OS mangling for the
variant dlopen() hoo-hah out there.
So here is a patch to define macros for the found support for dynamic
loading. I imagine that I picked stupid names for the macros, and would
welcome a correction. Otherwise, this seem OK?
-Fred
Index: acconfig.h
===
RCS file: /home/cvs/apr/acconfig.h,v
retrieving revision 1.41
diff -w -u -d -r1.41 acconfig.h
--- acconfig.h 2001/04/12 07:05:45 1.41
+++ acconfig.h 2001/04/18 21:45:03
@@ -29,6 +29,10 @@
#undef HAVE_GMTOFF
#undef USE_THREADS
+#undef DSO_USE_DLFCN
+#undef DSO_USE_SHL
+#undef DSO_USE_DYLD
+
#undef SIZEOF_SSIZE_T
#undef SIZEOF_SIZE_T
#undef SIZEOF_OFF_T
Index: configure.in
===
RCS file: /home/cvs/apr/configure.in,v
retrieving revision 1.292
diff -w -u -d -r1.292 configure.in
--- configure.in2001/04/18 17:46:39 1.292
+++ configure.in2001/04/18 21:45:03
@@ -780,19 +780,19 @@
[ --enable-dsoEnable dso support ],
[ tempdso=$enableval],
[
-AC_CHECK_FUNCS(NSLinkModule, [ tempdso=yes ], [ tempdso=no ])
+AC_CHECK_FUNCS(NSLinkModule, [ tempdso=dyld ], [ tempdso=no ])
if test $tempdso = no; then
- AC_CHECK_LIB(dl, dlopen, [ tempdso=yes LIBS=$LIBS -ldl ],
+ AC_CHECK_LIB(dl, dlopen, [ tempdso=dlfcn LIBS=$LIBS -ldl ],
tempdso=no)
fi
if test $tempdso = no; then
- AC_CHECK_FUNCS(dlopen, [ tempdso=yes ], [ tempdso=no ])
+ AC_CHECK_FUNCS(dlopen, [ tempdso=dlfcn ], [ tempdso=no ])
fi
if test $tempdso = no; then
AC_CHECK_LIB(root, load_image, tempdso=yes, tempdso=no)
fi
if test $tempdso = no; then
- AC_CHECK_LIB(dld, shl_load, [ tempdso=yes LIBS=$LIBS -ldld ],
+ AC_CHECK_LIB(dld, shl_load, [ tempdso=shl LIBS=$LIBS -ldld ],
tempdso=no)
fi
if test $tempdso = no; then
@@ -807,6 +807,11 @@
if test $tempdso = no; then
aprdso=0
else
+case $tempdso in
+ dlfcn) AC_DEFINE(DSO_USE_DLFCN);;
+ shl) AC_DEFINE(DSO_USE_SHL);;
+ dyld) AC_DEFINE(DSO_USE_DYLD);;
+esac
aprdso=1
apr_modules=$apr_modules dso
fi
Index: dso/unix/dso.c
===
RCS file: /home/cvs/apr/dso/unix/dso.c,v
retrieving revision 1.34
diff -w -u -d -r1.34 dso.c
--- dso/unix/dso.c 2001/04/18 17:47:10 1.34
+++ dso/unix/dso.c 2001/04/18 21:45:06
@@ -57,6 +57,10 @@
#if APR_HAS_DSO
+#if !defined(DSO_USE_DLFCN) !defined(DSO_USE_SHL)
!defined(DSO_USE_DYLD)
+#error No DSO implementation specified.
+#endif
+
#ifdef HAVE_STDDEF_H
#include stddef.h
#endif
@@ -71,11 +75,11 @@
if (dso-handle == NULL)
return APR_SUCCESS;
-#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
+#if defined(DSO_USE_SHL)
shl_unload((shl_t)dso-handle);
-#elif defined(DARWIN)
+#elif defined(DSO_USE_DYLD)
NSUnLinkModule(dso-handle, FALSE);
-#else
+#elif defined(DSO_USE_DLFCN)
if (dlclose(dso-handle) != 0)
return APR_EINIT;
#endif
@@ -87,10 +91,10 @@
APR_DECLARE(apr_status_t) apr_dso_load(apr_dso_handle_t **res_handle,
const char *path, apr_pool_t
*ctx)
{
-#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
+#if defined(DSO_USE_SHL)
shl_t os_handle = shl_load(path,
BIND_IMMEDIATE|BIND_VERBOSE|BIND_NOSTART, 0L);
-#elif defined(DARWIN)
+#elif defined(DSO_USE_DYLD)
NSObjectFileImage image;
NSModule os_handle;
char* err_msg = NULL;
@@ -107,24 +111,26 @@
#endif
}
-#elif defined(OSF1) || defined(SEQUENT) || defined(SNI) ||\
+#elif defined(DSO_USE_DLFCN)
+#if defined(OSF1) || defined(SEQUENT) || defined(SNI) ||\
(defined(__FreeBSD_version) (__FreeBSD_version = 22))
void *os_handle = dlopen((char *)path, RTLD_NOW | RTLD_GLOBAL);
#else
void *os_handle = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
#endif
+#endif /* DSO_USE_x */
*res_handle = apr_pcalloc(ctx, sizeof(**res_handle));
if(os_handle == NULL) {
-#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
+#if defined(DSO_USE_SHL)
(*res_handle)-errormsg = strerror(errno);
return errno;
-#elif defined(DARWIN)
+#elif defined(DSO_USE_DYLD)
(*res_handle)-errormsg = (err_msg) ? err_msg : link failed;
return APR_EDSOOPEN;
-#else
+#elif defined(DSO_USE_DLFCN)
(*res_handle)-errormsg = dlerror();
return APR_EDSOOPEN;
#endif
@@ -148,7 +154,7 @@
apr_dso_handle_t *handle,
Re: Clean up of DSO autoconfizification
On Wed, 18 Apr 2001, Wilfredo Sanchez wrote: I don't like how we detect whether DSO support should be enabled by testing for the possible implementation methods, and then rather than remembering which we found, we have all this #if THIS_OS crap all over dso/unix/dso.c. If we found dlfcn or shl or whatever, that should be what we conditionalize on, and maybe have per-OS mangling for the variant dlopen() hoo-hah out there. So here is a patch to define macros for the found support for dynamic loading. I imagine that I picked stupid names for the macros, and would welcome a correction. Otherwise, this seem OK? +1. The old code was lifted out of 1.3 directly. This is much easier to understand. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: Clean up of DSO autoconfizification
On Wed, Apr 18, 2001 at 02:56:02PM -0700, [EMAIL PROTECTED] wrote: On Wed, 18 Apr 2001, Wilfredo Sanchez wrote: I don't like how we detect whether DSO support should be enabled by testing for the possible implementation methods, and then rather than remembering which we found, we have all this #if THIS_OS crap all over dso/unix/dso.c. If we found dlfcn or shl or whatever, that should be what we conditionalize on, and maybe have per-OS mangling for the variant dlopen() hoo-hah out there. So here is a patch to define macros for the found support for dynamic loading. I imagine that I picked stupid names for the macros, and would welcome a correction. Otherwise, this seem OK? +1. The old code was lifted out of 1.3 directly. This is much easier to understand. +1, much better. I used an even cleaner solution in Python about a year and a half ago. At config time, I selected one of N files to compile and link in. Each module implemented a specific method. Here are the modules that Python has: dynload_aix.c dynload_hpux.c dynload_os2.cdynload_win.c dynload_beos.c dynload_mac.c dynload_shlib.c dynload_dl.cdynload_next.c dynload_stub.c That last is when dynload isn't available on a platform. It all works quite well, and the above kind of structure would actually work reasonably well in our setup. But hey... Let's go with Fred's patch now. The above is work that hasn't been done, and I'm not volunteering right now, so forget it :-) I just wanted to put out the brain-bug in case somebody wants to get motivated. Cheers, -g -- Greg Stein, http://www.lyra.org/
