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_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 ?
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 ?
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 ?
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 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 ?
> > 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 Tue, 17 Apr 2001, Greg Stein wrote: > 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 never said Perl couldn't be patched. but if i patch Perl (and i will), i'd still need to keep a duplicate version of the functionality in mod_perl until Perl 5.6.2 is released, which will be ages from now. > I'm not comfortable with APR being a workaround for other problems. its Apache-Portable-Runtime, it is supposed to solve portabilty problems. this is a portabilty problem. i am porting Perl to Apache, or Apache to Perl depending on how you look at it. there is something portable in apr that solves the problem but is private, all i want todo is expose it. > 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. > > Why are people crying about allocation performance? When you create a pool, > you get an 8k block. Allocations out of that first 8k are *FAST* (when you > go past it, then you need to get more from the system... maybe). And we're > talking about 8 (BeOS) to 16 (OS/2) bytes for these structures. You aren't > going to fill up that 8k of pool. > > 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.
Re: apr_dso_handle_close ?
On Tue, Apr 17, 2001 at 07:32:02AM -0700, [EMAIL PROTECTED] wrote: >... > b) With the model above, we are allocating everytime we want to fill out > an apr_dso_handle_t. That is a performance waste. We are allocating just > to immediately lose that memory, and re-allocate it. In fact, not only is > this a performance waste, it is a resource waste. You allocated 1 > sub-pool, and 3 apr_dso_handle_t's. My model uses one apr_dso_handle_t. > If the pool doesn't die immediately, then we have a bunch of wasted > memory. Wasted memory? 16 bytes each. See my other note. Talking about "wasted memory" is a non-starter. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: apr_dso_handle_close ?
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. Why are people crying about allocation performance? When you create a pool, you get an 8k block. Allocations out of that first 8k are *FAST* (when you go past it, then you need to get more from the system... maybe). And we're talking about 8 (BeOS) to 16 (OS/2) bytes for these structures. You aren't going to fill up that 8k of pool. 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. Cheers, -g On Tue, Apr 17, 2001 at 10:14:41AM -0700, Doug MacEachern wrote: > 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. > as you can see, the patch does not change any behavior, just exposes > an api to do the native close. > >... -- Greg Stein, http://www.lyra.org/
Re: apr_dso_handle_close ?
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.
as you can see, the patch does not change any behavior, just exposes
an api to do the native close.
Index: srclib/apr/dso/aix/dso.c
===
RCS file: /home/cvs/apr/dso/aix/dso.c,v
retrieving revision 1.14
diff -u -r1.14 dso.c
--- srclib/apr/dso/aix/dso.c2001/04/02 19:33:03 1.14
+++ srclib/apr/dso/aix/dso.c2001/04/17 17:08:07
@@ -132,12 +132,23 @@
* add the basic "wrappers" here.
*/
+APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
+{
+if (handle != NULL && dlclose(handle) != 0) {
+return APR_EINIT;
+}
+return APR_SUCCESS;
+}
+
static apr_status_t dso_cleanup(void *thedso)
{
+apr_status_t status;
apr_dso_handle_t *dso = thedso;
-if (dso->handle != NULL && dlclose(dso->handle) != 0)
-return APR_EINIT;
+if ((status = apr_dso_handle_close(dso->handle)) != APR_SUCCESS) {
+return status;
+}
+
dso->handle = NULL;
return APR_SUCCESS;
Index: srclib/apr/dso/beos/dso.c
===
RCS file: /home/cvs/apr/dso/beos/dso.c,v
retrieving revision 1.17
diff -u -r1.17 dso.c
--- srclib/apr/dso/beos/dso.c 2001/03/05 00:15:46 1.17
+++ srclib/apr/dso/beos/dso.c 2001/04/17 17:08:11
@@ -56,12 +56,23 @@
#if APR_HAS_DSO
+APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
+{
+if (handle != NULL && unload_add_on(handle) < B_NO_ERROR) {
+return APR_EINIT;
+}
+return APR_SUCCESS;
+}
+
static apr_status_t dso_cleanup(void *thedso)
{
+apr_status_t status;
apr_dso_handle_t *dso = thedso;
+
+if ((status = apr_dso_handle_close(dso->handle)) != APR_SUCCESS) {
+return status;
+}
-if (dso->handle != NULL && unload_add_on(dso->handle) < B_NO_ERROR)
- return APR_EINIT;
dso->handle = NULL;
return APR_SUCCESS;
Index: srclib/apr/dso/os2/dso.c
===
RCS file: /home/cvs/apr/dso/os2/dso.c,v
retrieving revision 1.21
diff -u -r1.21 dso.c
--- srclib/apr/dso/os2/dso.c2001/02/16 04:15:32 1.21
+++ srclib/apr/dso/os2/dso.c2001/04/17 17:08:14
@@ -59,20 +59,27 @@
#if APR_HAS_DSO
+APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
+{
+int rc = DosFreeModule(handle);
+return APR_OS2_STATUS(rc);
+}
+
static apr_status_t dso_cleanup(void *thedso)
{
+apr_status_t status;
apr_dso_handle_t *dso = thedso;
-int rc;
if (dso->handle == 0)
return APR_SUCCESS;
-rc = DosFreeModule(dso->handle);
+if ((status = apr_dso_handle_close(dso->handle)) != APR_SUCCESS) {
+return status;
+}
-if (rc == 0)
-dso->handle = 0;
+dso->handle = 0;
-return APR_OS2_STATUS(rc);
+return APR_SUCCESS;
}
Index: srclib/apr/dso/os390/dso.c
===
RCS file: /home/cvs/apr/dso/os390/dso.c,v
retrieving revision 1.8
diff -u -r1.8 dso.c
--- srclib/apr/dso/os390/dso.c 2001/02/16 04:15:33 1.8
+++ srclib/apr/dso/os390/dso.c 2001/04/17 17:08:18
@@ -59,22 +59,33 @@
#if APR_HAS_DSO
+APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
+{
+int rc;
+rc = dllfree(handle);
+
+if (rc == 0) {
+return APR_SUCCESS;
+}
+
+return errno;
+}
+
static apr_status_t dso_cleanup(void *thedso)
{
+apr_status_t status;
apr_dso_handle_t *dso = thedso;
-int rc;
if (dso->handle == 0)
return APR_SUCCESS;
-rc = dllfree(dso->handle);
-
-if (rc == 0) {
-dso->handle = 0;
-return APR_SUCCESS;
+if ((status = apr_dso_handle_close(dso->handle)) != APR_SUCCESS) {
+return status;
}
-dso->failing_errno = errno;
-return errno;
+
+dso->handle = 0;
+
+return APR_SUCCESS;
}
APR_DECLARE(apr_status_t) apr_dso_load(apr_dso_handle_t **res_handle,
Index: srclib/apr/dso/unix/dso.c
===
RCS file: /home/cvs/apr/dso/unix/dso.c,v
retrieving revision 1.33
diff -u -r1.33 dso.c
--- srclib/apr/dso/unix/dso.c 2001/02/16 21:04:17 1.33
+++ srclib/apr/dso/unix/dso.c 2001/04/17 17:08:21
@@ -64,19 +64,30 @@
#include /* for strerror() on HP-UX */
#endif
+APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
+{
+#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
+shl_unload((shl_t)dso->handle);
+#else
+if (dlclose(handle) != 0) {
+return APR_EINIT;
+}
+#endif
+return APR_SUCCESS;
+}
+
static apr_status_t dso_cleanup(void *thedso)
{
+apr_status_t status;
apr_dso_handle_t *dso = thedso;
if (dso->hand
Re: apr_dso_handle_close ?
On Mon, 16 Apr 2001 [EMAIL PROTECTED] wrote:
> My biggest problem with passing in a void *, and using it as the dlhandle
> to close, is that it won't work in error cases on some platforms. OS/2
> requires that we save the error message when the error happens, at least
> that is my understanding. This means that if there is an error, we will
> need to store that string someplace.
wait a sec, this is true for the load, but not unload, os2/dso.c:
static apr_status_t dso_cleanup(void *thedso)
{
apr_dso_handle_t *dso = thedso;
int rc;
if (dso->handle == 0)
return APR_SUCCESS;
rc = DosFreeModule(dso->handle);
if (rc == 0)
dso->handle = 0;
return APR_OS2_STATUS(rc);
}
i still very much want to avoid using a pool to close these handles.
Re: apr_dso_handle_close ?
> Let's just have: > > apr_dso_make(apr_dso_handle_t **h, void *plat_hand, apr_pool_t *p); > > I know that Doug wants to do something like: > > apr_dso_make(&h, p); > apr_dso_fill(h, handle1); > apr_dso_unload(h); > apr_dso_fill(h, handle2); > apr_dso_unload(h); > apr_dso_fill(h, handle3); > apr_dso_unload(h); > > But I'd say: > > sub = apr_create_pool(p); > apr_dso_make(&h, handle1, sub); > apr_dso_unload(h); > apr_dso_make(&h, handle2, sub); > apr_dso_unload(h); > apr_dso_make(&h, handle3, sub); > apr_dso_unload(h); > apr_pool_destroy(sub); > > That keeps our API simpler. If somebody doesn't want to care about memory so > much, then the subpool isn't even needed. As I mentioned before, if you're > tossing DSO's, I bet there is a pool that is getting tossed, too (so the > apr_dso_make memory can go in there for a short period). a) I disagree that just because something is being unloaded, a pool is going away. > Heck, I just realized. This is even cleaner: > > sub = apr_create_pool(p); > apr_dso_make(&h, handle1, sub); > apr_dso_make(&h, handle2, sub); > apr_dso_make(&h, handle3, sub); > apr_pool_destroy(sub); > > Since DSO's go away with the pool they're registered in, the above will toss > all the DSOs at pool destruction. Of course, you don't get error feedback > from each unload(), but it all depends on whether you even *want* it. > > Anyhow... let's have just a single apr_dso_make(). I don't need a need for > two functions. b) With the model above, we are allocating everytime we want to fill out an apr_dso_handle_t. That is a performance waste. We are allocating just to immediately lose that memory, and re-allocate it. In fact, not only is this a performance waste, it is a resource waste. You allocated 1 sub-pool, and 3 apr_dso_handle_t's. My model uses one apr_dso_handle_t. If the pool doesn't die immediately, then we have a bunch of wasted memory. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: apr_dso_handle_close ?
Let's just have: apr_dso_make(apr_dso_handle_t **h, void *plat_hand, apr_pool_t *p); I know that Doug wants to do something like: apr_dso_make(&h, p); apr_dso_fill(h, handle1); apr_dso_unload(h); apr_dso_fill(h, handle2); apr_dso_unload(h); apr_dso_fill(h, handle3); apr_dso_unload(h); But I'd say: sub = apr_create_pool(p); apr_dso_make(&h, handle1, sub); apr_dso_unload(h); apr_dso_make(&h, handle2, sub); apr_dso_unload(h); apr_dso_make(&h, handle3, sub); apr_dso_unload(h); apr_pool_destroy(sub); That keeps our API simpler. If somebody doesn't want to care about memory so much, then the subpool isn't even needed. As I mentioned before, if you're tossing DSO's, I bet there is a pool that is getting tossed, too (so the apr_dso_make memory can go in there for a short period). Heck, I just realized. This is even cleaner: sub = apr_create_pool(p); apr_dso_make(&h, handle1, sub); apr_dso_make(&h, handle2, sub); apr_dso_make(&h, handle3, sub); apr_pool_destroy(sub); Since DSO's go away with the pool they're registered in, the above will toss all the DSOs at pool destruction. Of course, you don't get error feedback from each unload(), but it all depends on whether you even *want* it. Anyhow... let's have just a single apr_dso_make(). I don't need a need for two functions. Cheers, -g On Mon, Apr 16, 2001 at 03:03:11PM -0700, [EMAIL PROTECTED] wrote: > > Okay, this issue has died down now, so it is time to bring it back up, > with a potential solution. Doug and I have already discussed this, and he > agrees that this solves his problem. It also solves the other problems > that have been raised on this list. > > My biggest problem with passing in a void *, and using it as the dlhandle > to close, is that it won't work in error cases on some platforms. OS/2 > requires that we save the error message when the error happens, at least > that is my understanding. This means that if there is an error, we will > need to store that string someplace. > > My solution: > >two functions: > apr_dso_make(apr_dso_handle_t **h, apr_pool_t *p); > Allocate enough space for an apr_dso_handle_t > apr_dso_fill(apr_dso_handle_t *h, void *plat_hand); > Fill the handle out. > >Now, programs can safely use apr_dso_close on the resulting > apr_dso_handle_t, and we don't lose portability. They can also > re-use the original apr_dso_handle_t until they are done closing > DSOs. There is a large part of me that would like to see us use this > model for all of the portability code that we have. > > Unless there are any arguments, I will be implementing this on some > platforms tomorrow sometime. > > Ryan > > > On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > > > > The problem is that the code you want to implement is incompatible with > > APR's dso code. In order to use the function that you want, you would > > need to start by using the system's dlopen. If you are going to use the > > systems dlopen, then you should use the systems dlclose. > > > > What we really need, I guess, is apr_dso_put and apr_dso_get. That would > > follow the model that we currently have, instead of creating another > > function that would take the system type directly. > > > > Ryan > > > > On Mon, 9 Apr 2001, Doug MacEachern wrote: > > > > > On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > > > > > > > > > > How is this different from apr_dso_unload? > > > > > > apr_dso_unload() takes an apr_dso_handle_t argument, so i would have todo > > > something ugly like: > > > > > > apr_dso_handle_t dso; > > > dso.handle = (void*)handle_from_perl; > > > dso.cont = p; > > > apr_dso_unload(&dso); > > > > > > which i couldn't do if i wanted to, since apr_dso_handle_t is an > > > incomplete type. > > > > > > > > > > > > > > > ___ > > Ryan Bloom [EMAIL PROTECTED] > > 406 29th St. > > San Francisco, CA 94131 > > --- > > > > > > > ___ > Ryan Bloom[EMAIL PROTECTED] > 406 29th St. > San Francisco, CA 94131 > --- -- Greg Stein, http://www.lyra.org/
Re: apr_dso_handle_close ?
Okay, this issue has died down now, so it is time to bring it back up, with a potential solution. Doug and I have already discussed this, and he agrees that this solves his problem. It also solves the other problems that have been raised on this list. My biggest problem with passing in a void *, and using it as the dlhandle to close, is that it won't work in error cases on some platforms. OS/2 requires that we save the error message when the error happens, at least that is my understanding. This means that if there is an error, we will need to store that string someplace. My solution: two functions: apr_dso_make(apr_dso_handle_t **h, apr_pool_t *p); Allocate enough space for an apr_dso_handle_t apr_dso_fill(apr_dso_handle_t *h, void *plat_hand); Fill the handle out. Now, programs can safely use apr_dso_close on the resulting apr_dso_handle_t, and we don't lose portability. They can also re-use the original apr_dso_handle_t until they are done closing DSOs. There is a large part of me that would like to see us use this model for all of the portability code that we have. Unless there are any arguments, I will be implementing this on some platforms tomorrow sometime. Ryan On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > The problem is that the code you want to implement is incompatible with > APR's dso code. In order to use the function that you want, you would > need to start by using the system's dlopen. If you are going to use the > systems dlopen, then you should use the systems dlclose. > > What we really need, I guess, is apr_dso_put and apr_dso_get. That would > follow the model that we currently have, instead of creating another > function that would take the system type directly. > > Ryan > > On Mon, 9 Apr 2001, Doug MacEachern wrote: > > > On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > > > > > > > How is this different from apr_dso_unload? > > > > apr_dso_unload() takes an apr_dso_handle_t argument, so i would have todo > > something ugly like: > > > > apr_dso_handle_t dso; > > dso.handle = (void*)handle_from_perl; > > dso.cont = p; > > apr_dso_unload(&dso); > > > > which i couldn't do if i wanted to, since apr_dso_handle_t is an > > incomplete type. > > > > > > > > > ___ > Ryan Bloom[EMAIL PROTECTED] > 406 29th St. > San Francisco, CA 94131 > --- > > ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: apr_dso_handle_close ?
On Tue, 10 Apr 2001, Greg Stein wrote: > I'm with Ryan. We don't need to write a cover for the underlying DSO close. > If you use APR's DSO stuff, then you can use its closing system. we're not writing anything, just exposing functionality that is already in there, tucked away private. > Go patch Perl to add a cover for dlclose() since that is where you got the > handle. that is not useful for the moment, it will months before a new Perl is released. granted, that is not apr's problem, but apr can provide a solution much sooner. > I'd be fine with apr_dso_make(void *handle). [ back to the put vs make > discussion ] > > And piffle with the extra memory or speed with wrapping an apr_dso structure > around the handle [just to close it]. You're loading/unloading DSOs for > chrissakes. Perf just doesn't come into the picture. And I'm sure you've got > a pool that the apr_dso can go into, which is just about to get tossed after > you're done unloading DSOs. i am also closing these handles at request time, if an interpreter clone is knocked off due to PerlInterpMaxRequest or PerlInterpMaxSpare. don't forget, if a pool runs out of room, there's a mutex lock on alloc_mutex and malloc of 8k or whatever the default block size is. 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. if just the close part was exposed, i could do all this without using a pool and pile of extra function calls. it is a waste of resources, period. a waste of resources i can live with maybe, but don't tell me "perf just doesn't come into the picture". forget it, i will copy-n-paste my own portabilty layer together, patch Perl and wait a year or two till i can throw away the duplication.
Re: apr_dso_handle_close ?
I'm with Ryan. We don't need to write a cover for the underlying DSO close. If you use APR's DSO stuff, then you can use its closing system. Go patch Perl to add a cover for dlclose() since that is where you got the handle. I'd be fine with apr_dso_make(void *handle). [ back to the put vs make discussion ] And piffle with the extra memory or speed with wrapping an apr_dso structure around the handle [just to close it]. You're loading/unloading DSOs for chrissakes. Perf just doesn't come into the picture. And I'm sure you've got a pool that the apr_dso can go into, which is just about to get tossed after you're done unloading DSOs. -1 on apr_dso_handle_close. +0 on apr_dso_make. -0 on apr_dso_put. Cheers, -g On Mon, Apr 09, 2001 at 08:54:45PM -0700, [EMAIL PROTECTED] wrote: > > The problem is that the code you want to implement is incompatible with > APR's dso code. In order to use the function that you want, you would > need to start by using the system's dlopen. If you are going to use the > systems dlopen, then you should use the systems dlclose. > > What we really need, I guess, is apr_dso_put and apr_dso_get. That would > follow the model that we currently have, instead of creating another > function that would take the system type directly. > > Ryan > > On Mon, 9 Apr 2001, Doug MacEachern wrote: > > > On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > > > > > > > How is this different from apr_dso_unload? > > > > apr_dso_unload() takes an apr_dso_handle_t argument, so i would have todo > > something ugly like: > > > > apr_dso_handle_t dso; > > dso.handle = (void*)handle_from_perl; > > dso.cont = p; > > apr_dso_unload(&dso); > > > > which i couldn't do if i wanted to, since apr_dso_handle_t is an > > incomplete type. > > > > > > > > > ___ > Ryan Bloom[EMAIL PROTECTED] > 406 29th St. > San Francisco, CA 94131 > --- -- Greg Stein, http://www.lyra.org/
Re: apr_dso_handle_close ?
On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > The problem is that the code you want to implement is incompatible with > APR's dso code. In order to use the function that you want, you would > need to start by using the system's dlopen. If you are going to use the > systems dlopen, then you should use the systems dlclose. Perl already takes care of calling the right dlopen-ish function for the platform, which would be exactly the same as what apr_dso_load() would call. if you look at the patch i sent, it just exposes the part which would call the system's dlclose, passing it what the system's dlopen returned (same as what dso->handle would be if opened with apr_dso_load). > What we really need, I guess, is apr_dso_put and apr_dso_get. That would > follow the model that we currently have, instead of creating another > function that would take the system type directly. something like apr_dso_put would probably work, but having to allocate the apr_dso_handle_t and space for the registered cleanup is a waste. when Perl does its dlopen, it will not be calling apr_dso_put. mod_perl needs to do the dlclose inside of a registered cleanup. in otherwords, apr_dso_put in this case would be registering a cleanup inside of a cleanup. the patch would not change the behavior of the current apr_dso_* functions, it just exposes the dlclose mapping so mod_perl doesn't need to duplicate it.
Re: apr_dso_handle_close ?
The problem is that the code you want to implement is incompatible with APR's dso code. In order to use the function that you want, you would need to start by using the system's dlopen. If you are going to use the systems dlopen, then you should use the systems dlclose. What we really need, I guess, is apr_dso_put and apr_dso_get. That would follow the model that we currently have, instead of creating another function that would take the system type directly. Ryan On Mon, 9 Apr 2001, Doug MacEachern wrote: > On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > > > > How is this different from apr_dso_unload? > > apr_dso_unload() takes an apr_dso_handle_t argument, so i would have todo > something ugly like: > > apr_dso_handle_t dso; > dso.handle = (void*)handle_from_perl; > dso.cont = p; > apr_dso_unload(&dso); > > which i couldn't do if i wanted to, since apr_dso_handle_t is an > incomplete type. > > > ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: apr_dso_handle_close ?
On Mon, 9 Apr 2001 [EMAIL PROTECTED] wrote: > > How is this different from apr_dso_unload? apr_dso_unload() takes an apr_dso_handle_t argument, so i would have todo something ugly like: apr_dso_handle_t dso; dso.handle = (void*)handle_from_perl; dso.cont = p; apr_dso_unload(&dso); which i couldn't do if i wanted to, since apr_dso_handle_t is an incomplete type.
Re: apr_dso_handle_close ?
How is this different from apr_dso_unload?
Ryan
On Mon, 9 Apr 2001, Doug MacEachern wrote:
> mod_perl needs to close the handle for each Perl extension .so loaded by
> Perl modules. in 1.x we could call:
> ap_os_dso_unload(handle);
>
> where handle is the same value returned by dlopen() or similar.
> Perl's abstraction is not an option since you need to call a Perl function
> to use it, and we cannot close these handles until the interpreter is
> destroyed.
>
> something like the following for each $os/dso.c would do the trick.
> i will implement the patch if there are no objections, otherwise it will
> really suck to duplicate this functionality that is currently private in
> apr.
>
> Index: srclib/apr/dso/unix/dso.c
> ===
> RCS file: /home/cvs/apr/dso/unix/dso.c,v
> retrieving revision 1.33
> diff -u -r1.33 dso.c
> --- srclib/apr/dso/unix/dso.c 2001/02/16 21:04:17 1.33
> +++ srclib/apr/dso/unix/dso.c 2001/04/10 00:08:52
> @@ -64,19 +64,30 @@
> #include /* for strerror() on HP-UX */
> #endif
>
> +APR_DECLARE(apr_status_t) apr_dso_handle_close(void *handle)
> +{
> +#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
> +shl_unload((shl_t)dso->handle);
> +#else
> +if (dlclose(handle) != 0) {
> +return APR_EINIT;
> +}
> +#endif
> +return APR_SUCCESS;
> +}
> +
> static apr_status_t dso_cleanup(void *thedso)
> {
> +apr_status_t status;
> apr_dso_handle_t *dso = thedso;
>
> if (dso->handle == NULL)
> return APR_SUCCESS;
>
> -#if defined(HPUX) || defined(HPUX10) || defined(HPUX11)
> -shl_unload((shl_t)dso->handle);
> -#else
> -if (dlclose(dso->handle) != 0)
> -return APR_EINIT;
> -#endif
> +if ((status = apr_dso_handle_close(dso->handle)) != APR_SUCCESS) {
> +return status;
> +}
> +
> dso->handle = NULL;
>
> return APR_SUCCESS;
>
>
___
Ryan Bloom [EMAIL PROTECTED]
406 29th St.
San Francisco, CA 94131
---
