This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
bugs in resource management in gconv_db
- To: libc-alpha at sources dot redhat dot com
- Subject: bugs in resource management in gconv_db
- From: Bruno Haible <haible at ilog dot fr>
- Date: Mon, 4 Sep 2000 14:55:28 +0200 (CEST)
The management of per-step initializer and destructor in gconv_db.c
currently has the following bugs:
- In gen_steps(), when an initializer fails, not only the previously
initialized steps will be destructed, also the failed step itself.
- In gen_steps(), when an initializer fails and some previously initialized
module is a built-in one, __gconv_release_shlib (NULL) will be called,
thus aging all recently unloaded modules.
- In increment_counter(), after an unexpected loading failure,
__gconv_release_shlib is called without calling the destructor of the
previously initialized step.
- In increment_counter(), after reloading a module, the destructor of a
step is called - but the step was initialized before reloading, thus
in a partially different address space. But a module should be able to
use addresses in its own data.
- In increment_counter(), the initializer of a step is not called if it's
a builtin module. But the BUILTIN_TRANSFORMATION foresees the case where
builtin modules have initializers.
- In __gconv_close_transform(), __gconv_release_shlib is called without
calling the destructor of the previously initialized step.
This patch fixes them all. The __gconv_release_shlib return value is changed
to void; most calls did already ignore it. Steps are initialized if and
only if __counter > 0, because if the __counter is zero, there is no
guarantee that the module is still in the address space and will be able
to destruct the step.
2000-09-03 Bruno Haible <haible@clisp.cons.org>
* iconv/gconv_int.h (__gconv_release_shlib): Change return type to
void.
* iconv/gconv_dl.c (do_release_shlib): Don't decrement the counter
below -TRIES_BEFORE_UNLOAD-1, to avoid wraparound.
(__gconv_release_shlib): Change return type to void.
* iconv/gconv_builtin.c (__gconv_get_builtin_trans): Don't set
step->__counter here.
* iconv/gconv_db.c (free_derivation): Don't call a step's destructor
if the reference is zero.
(release_step): New function.
(gen_steps): Always initialize the __counter to 1. Use release_step.
Don't call the destructor on the step whose initializer failed.
(increment_counter): Use release_step. Don't normally run destructors
here.
(__gconv_close_transform): Use release_step.
*** glibc-20000831/iconv/gconv_int.h.bak Mon Jul 3 16:39:26 2000
--- glibc-20000831/iconv/gconv_int.h Sat Sep 2 22:17:31 2000
***************
*** 55,61 ****
object is also handled. */
struct __gconv_loaded_object
{
! /* Name of the object. */
const char *name;
/* Reference counter for the db functionality. If no conversion is
--- 55,61 ----
object is also handled. */
struct __gconv_loaded_object
{
! /* Name of the object. It must be the first structure element. */
const char *name;
/* Reference counter for the db functionality. If no conversion is
***************
*** 201,207 ****
/* Release shared object. If no further reference is available unload
the object. */
! extern int __gconv_release_shlib (struct __gconv_loaded_object *handle)
internal_function;
/* Fill STEP with information about builtin module with NAME. */
--- 201,207 ----
/* Release shared object. If no further reference is available unload
the object. */
! extern void __gconv_release_shlib (struct __gconv_loaded_object *handle)
internal_function;
/* Fill STEP with information about builtin module with NAME. */
*** glibc-20000831/iconv/gconv_dl.c.bak Tue Jun 13 18:38:30 2000
--- glibc-20000831/iconv/gconv_dl.c Sat Sep 2 22:18:46 2000
***************
*** 164,170 ****
}
else if (obj->counter <= 0)
{
! if (--obj->counter < -TRIES_BEFORE_UNLOAD && obj->handle != NULL)
{
/* Unload the shared object. */
__libc_dlclose (obj->handle);
--- 164,172 ----
}
else if (obj->counter <= 0)
{
! if (obj->counter >= -TRIES_BEFORE_UNLOAD)
! --obj->counter;
! if (obj->counter < -TRIES_BEFORE_UNLOAD && obj->handle != NULL)
{
/* Unload the shared object. */
__libc_dlclose (obj->handle);
***************
*** 175,181 ****
/* Notify system that a shared object is not longer needed. */
! int
internal_function
__gconv_release_shlib (struct __gconv_loaded_object *handle)
{
--- 177,183 ----
/* Notify system that a shared object is not longer needed. */
! void
internal_function
__gconv_release_shlib (struct __gconv_loaded_object *handle)
{
***************
*** 186,193 ****
with release counts <= 0. This way we can finally unload them
if necessary. */
__twalk (loaded, do_release_shlib);
-
- return __GCONV_OK;
}
--- 188,193 ----
*** glibc-20000831/iconv/gconv_builtin.c.bak Thu Jul 13 19:45:50 2000
--- glibc-20000831/iconv/gconv_builtin.c Sun Sep 3 19:52:02 2000
***************
*** 75,81 ****
step->__fct = map[cnt].fct;
step->__init_fct = map[cnt].init;
step->__end_fct = map[cnt].end;
- step->__counter = INT_MAX;
step->__shlib_handle = NULL;
step->__modname = NULL;
--- 75,80 ----
*** glibc-20000831/iconv/gconv_db.c.bak Sat Sep 2 19:35:55 2000
--- glibc-20000831/iconv/gconv_db.c Sun Sep 3 19:59:02 2000
***************
*** 163,169 ****
size_t cnt;
for (cnt = 0; cnt < deriv->nsteps; ++cnt)
! if (deriv->steps[cnt].__end_fct)
DL_CALL_FCT (deriv->steps[cnt].__end_fct, (&deriv->steps[cnt]));
/* Free the name strings. */
--- 163,170 ----
size_t cnt;
for (cnt = 0; cnt < deriv->nsteps; ++cnt)
! if (deriv->steps[cnt].__counter > 0
! && deriv->steps[cnt].__end_fct != NULL)
DL_CALL_FCT (deriv->steps[cnt].__end_fct, (&deriv->steps[cnt]));
/* Free the name strings. */
***************
*** 175,180 ****
--- 176,203 ----
}
+ /* Decrement the reference count for a single step in a steps array. */
+ static inline void
+ release_step (struct __gconv_step *step)
+ {
+ if (--step->__counter == 0)
+ {
+ /* Call the destructor. */
+ if (step->__end_fct != NULL)
+ DL_CALL_FCT (step->__end_fct, (step));
+
+ #ifndef STATIC_GCONV
+ /* Skip builtin modules; they are not reference counted. */
+ if (step->__shlib_handle != NULL)
+ {
+ /* Release the loaded module. */
+ __gconv_release_shlib (step->__shlib_handle);
+ step->__shlib_handle = NULL;
+ }
+ #endif
+ }
+ }
+
static int
internal_function
gen_steps (struct derivation_step *best, const char *toset,
***************
*** 222,228 ****
result[step_cnt].__shlib_handle = shlib_handle;
result[step_cnt].__modname = shlib_handle->name;
- result[step_cnt].__counter = 1;
result[step_cnt].__fct = shlib_handle->fct;
result[step_cnt].__init_fct = shlib_handle->init_fct;
result[step_cnt].__end_fct = shlib_handle->end_fct;
--- 245,250 ----
***************
*** 233,238 ****
--- 255,262 ----
__gconv_get_builtin_trans (current->code->module_name,
&result[step_cnt]);
+ result[step_cnt].__counter = 1;
+
/* Call the init function. */
result[step_cnt].__data = NULL;
if (result[step_cnt].__init_fct != NULL)
***************
*** 245,250 ****
--- 269,275 ----
failed = 1;
/* Make sure we unload this modules. */
--step_cnt;
+ result[step_cnt].__end_fct = NULL;
break;
}
}
***************
*** 256,268 ****
{
/* Something went wrong while initializing the modules. */
while (++step_cnt < *nsteps)
! {
! if (result[step_cnt].__end_fct != NULL)
! DL_CALL_FCT (result[step_cnt].__end_fct, (&result[step_cnt]));
! #ifndef STATIC_GCONV
! __gconv_release_shlib (result[step_cnt].__shlib_handle);
! #endif
! }
free (result);
*nsteps = 0;
*handle = NULL;
--- 281,287 ----
{
/* Something went wrong while initializing the modules. */
while (++step_cnt < *nsteps)
! release_step (&result[step_cnt]);
free (result);
*nsteps = 0;
*handle = NULL;
***************
*** 292,320 ****
int result = __GCONV_OK;
while (cnt-- > 0)
! if (steps[cnt].__counter++ == 0)
! {
! steps[cnt].__shlib_handle =
! __gconv_find_shlib (steps[cnt].__modname);
! if (steps[cnt].__shlib_handle == NULL)
! {
! /* Oops, this is the second time we use this module (after
! unloading) and this time loading failed!? */
! while (++cnt < nsteps)
! __gconv_release_shlib (steps[cnt].__shlib_handle);
! result = __GCONV_NOCONV;
! break;
! }
!
! steps[cnt].__init_fct = steps[cnt].__shlib_handle->init_fct;
! steps[cnt].__fct = steps[cnt].__shlib_handle->fct;
! steps[cnt].__end_fct = steps[cnt].__shlib_handle->end_fct;
!
! if (steps[cnt].__end_fct != NULL)
! DL_CALL_FCT (steps[cnt].__end_fct, (&steps[cnt]));
! if (steps[cnt].__init_fct != NULL)
! DL_CALL_FCT (steps[cnt].__init_fct, (&steps[cnt]));
! }
return result;
}
#endif
--- 311,348 ----
int result = __GCONV_OK;
while (cnt-- > 0)
! {
! struct __gconv_step *step = &steps[cnt];
!
! if (step->__counter++ == 0)
! {
! /* Skip builtin modules. */
! if (step->__modname != NULL)
! {
! /* Reopen a previously used module. */
! step->__shlib_handle = __gconv_find_shlib (step->__modname);
! if (step->__shlib_handle == NULL)
! {
! /* Oops, this is the second time we use this module
! (after unloading) and this time loading failed!? */
! --step->__counter;
! while (++cnt < nsteps)
! release_step (&steps[cnt]);
! result = __GCONV_NOCONV;
! break;
! }
!
! /* The function addresses defined by the module may
! have changed. */
! step->__fct = step->__shlib_handle->fct;
! step->__init_fct = step->__shlib_handle->init_fct;
! step->__end_fct = step->__shlib_handle->end_fct;
! }
!
! if (step->__init_fct != NULL)
! DL_CALL_FCT (step->__init_fct, (step));
! }
! }
return result;
}
#endif
***************
*** 609,622 ****
__libc_lock_lock (lock);
while (nsteps-- > 0)
! if (steps[nsteps].__shlib_handle != NULL
! && --steps[nsteps].__counter == 0)
! {
! result = __gconv_release_shlib (steps[nsteps].__shlib_handle);
! if (result != __GCONV_OK)
! break;
! steps[nsteps].__shlib_handle = NULL;
! }
/* Release the lock. */
__libc_lock_unlock (lock);
--- 695,701 ----
__libc_lock_lock (lock);
while (nsteps-- > 0)
! release_step (&steps[nsteps]);
/* Release the lock. */
__libc_lock_unlock (lock);