httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stodd...@apache.org
Subject cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c
Date Thu, 07 Mar 2002 21:44:49 GMT
stoddard    02/03/07 13:44:49

  Modified:    modules/experimental mod_mem_cache.c
  Log:
  Do a better job of cleaning up (plug memory leaks) and handling aborted
  cache updates. We really need a better way to allocate cache_objects.
  Making WAY too many malloc/calloc calls...
  
  Revision  Changes    Path
  1.25      +75 -51    httpd-2.0/modules/experimental/mod_mem_cache.c
  
  Index: mod_mem_cache.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
  retrieving revision 1.24
  retrieving revision 1.25
  diff -u -r1.24 -r1.25
  --- mod_mem_cache.c	7 Mar 2002 19:27:58 -0000	1.24
  +++ mod_mem_cache.c	7 Mar 2002 21:44:49 -0000	1.25
  @@ -136,34 +136,54 @@
   {
       mem_cache_object_t *mobj = obj->vobj;
   
  +    /* TODO:
  +     * We desperately need a more efficient way of allocating objects. We're
  +     * making way too many malloc calls to create a fully populated 
  +     * cache object...
  +     */
  +
       /* Cleanup the cache_object_t */
       if (obj->key) {
           free(obj->key);
       }
  +    if (obj->info.content_type) {
  +        free(obj->info.content_type);
  +    }
  +    if (obj->info.etag) {
  +        free(obj->info.etag);
  +    }
  +    if (obj->info.lastmods) {
  +        free(obj->info.lastmods);
  +    }
  +    if (obj->info.filename) {
  +        free(obj->info.filename);
  +    }
  +
       free(obj);
       
       /* Cleanup the mem_cache_object_t */
  -    if (!mobj) {
  -        return;
  -    }
  -    if (mobj->m) {
  -        free(mobj->m);
  -    }
  -    /* XXX should freeing of the info be done here or in cache_storage ? 
  -    if (obj->info.content_type ) {
  -        free((char*)obj->info.content_type );
  -        obj->info.content_type =NULL;
  -    }
  -    if (obj->info.filename ) {
  -        free( (char*)obj->info.filename );
  -        obj->info.filename= NULL;
  -    }
  -    */
  -    /* XXX Cleanup the headers */
  -    if (mobj->num_header_out) {
  -        
  +    if (mobj) {
  +        if (mobj->m) {
  +            free(mobj->m);
  +        }
  +        if (mobj->header_out) {
  +            if (mobj->header_out[0].hdr) 
  +                free(mobj->header_out[0].hdr);
  +            free(mobj->header_out);
  +        }
  +        if (mobj->subprocess_env) {
  +            if (mobj->subprocess_env[0].hdr) 
  +                free(mobj->subprocess_env[0].hdr);
  +            free(mobj->subprocess_env);
  +        }
  +        if (mobj->notes) {
  +            if (mobj->notes[0].hdr) 
  +                free(mobj->notes[0].hdr);
  +            free(mobj->notes);
  +        }
  +        free(mobj);
       }
  -    free(mobj);
  +    return;
   }
   static apr_status_t decrement_refcount(void *arg) 
   {
  @@ -278,35 +298,29 @@
       }
   
       /* Allocate and initialize cache_object_t */
  -    obj = malloc(sizeof(*obj));
  +    obj = calloc(1, sizeof(*obj));
       if (!obj) {
           return DECLINED;
       }
  -    memset(obj,'\0', sizeof(*obj));
  -    obj->key = malloc(strlen(key) + 1);
  +    obj->key = calloc(1, strlen(key) + 1);
       if (!obj->key) {
  -        free(obj);
  +        cleanup_cache_object(obj);
           return DECLINED;
       }
       strncpy(obj->key, key, strlen(key) + 1);
       obj->info.len = len;
  -    obj->complete = 0;   /* Cache object is not complete */
   
   
       /* Allocate and init mem_cache_object_t */
  -    mobj = malloc(sizeof(*mobj));
  +    mobj = calloc(1, sizeof(*mobj));
       if (!mobj) {
  -        /* XXX: Cleanup */
           cleanup_cache_object(obj);
  +        return DECLINED;
       }
  -    memset(mobj,'\0', sizeof(*mobj));
  -    obj->vobj = mobj;    /* Reference the mem_cache_object_t out of 
  -                          * cache_object_t 
  -                          */
  -    mobj->refcount = 0;
  -    mobj->cleanup = 0;
  -    mobj->m_len = len;    /* Duplicates info in cache_object_t info */
   
  +    /* Reference mem_cache_object_t out of cache_object_t */
  +    obj->vobj = mobj;
  +    mobj->m_len = len;
   
       /* Place the cache_object_t into the hash table.
        * Note: Perhaps we should wait to put the object in the
  @@ -332,6 +346,14 @@
           apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), obj);
           sconf->object_cnt++;
           sconf->cache_size += len;
  +        /* Call a cleanup at the end of the request to garbage collect
  +         * a partially completed (aborted) cache update.
  +         */
  +        obj->complete = 0;
  +        mobj->refcount = 1;
  +        mobj->cleanup = 1;
  +        apr_pool_cleanup_register(r->pool, obj, decrement_refcount, 
  +                                  apr_pool_cleanup_null);
       }
       if (sconf->lock) {
           apr_thread_mutex_unlock(sconf->lock);
  @@ -373,15 +395,20 @@
       
       if (obj) {
           mem_cache_object_t *mobj = (mem_cache_object_t *) obj->vobj;
  -        mobj->refcount++;
  -        apr_pool_cleanup_register(r->pool, obj, decrement_refcount, apr_pool_cleanup_null);
  +        if (obj->complete) {
  +            mobj->refcount++;
  +            apr_pool_cleanup_register(r->pool, obj, decrement_refcount, apr_pool_cleanup_null);
  +        }
  +        else {
  +            obj = NULL;
  +        }
       }
   
       if (sconf->lock) {
           apr_thread_mutex_unlock(sconf->lock);
       }
   
  -    if (!obj || !(obj->complete)) {
  +    if (!obj) {
           return DECLINED;
       }
   
  @@ -439,9 +466,8 @@
          *obj=NULL;
          return OK;
      }
  -    *obj = malloc(sizeof(cache_header_tbl_t) * table->a.nelts);
  +    *obj = calloc(1, sizeof(cache_header_tbl_t) * table->a.nelts);
       if (NULL == *obj) {
  -        /* cleanup_cache_obj(h->cache_obj); */
           return DECLINED;
       }
       for (i = 0; i < table->a.nelts; ++i) {
  @@ -451,11 +477,9 @@
       }
   
       /* Transfer the headers into a contiguous memory block */
  -    buf = malloc(len);
  +    buf = calloc(1, len);
       if (!buf) {
  -        free(obj);
           *obj = NULL;
  -        /* cleanup_cache_obj(h->cache_obj); */
           return DECLINED;
       }
   
  @@ -471,7 +495,6 @@
           idx+=len;
       }
       return OK;
  - 
   }
   static int unserialize_table( cache_header_tbl_t *ctbl, 
                                 int num_headers, 
  @@ -588,7 +611,6 @@
       if (rc != OK ) {
           return rc;
       }
  -
    
       /* Init the info struct */
       if (info->date) {
  @@ -601,18 +623,15 @@
           obj->info.expire = info->expire;
       }
       if (info->content_type) {
  -        obj->info.content_type = (char*) malloc(strlen(info->content_type) + 1);
  +        obj->info.content_type = (char*) calloc(1, strlen(info->content_type) + 1);
           if (!obj->info.content_type) {
  -            /* cleanup the object? */
               return DECLINED;
           }
           strcpy((char*) obj->info.content_type, info->content_type);
       }
       if ( info->filename) {
  -        obj->info.filename = (char*) malloc(strlen(info->filename )+1);
  +        obj->info.filename = (char*) calloc(1, strlen(info->filename) + 1);
           if (!obj->info.filename ) {
  -            free( (char*)obj->info.content_type );
  -            obj->info.content_type =NULL;
               return DECLINED;
           }
           strcpy((char*) obj->info.filename, info->filename );
  @@ -636,7 +655,7 @@
       if (mobj->m == NULL) {
           mobj->m = malloc(mobj->m_len);
           if (mobj->m == NULL) {
  -            /* Cleanup cache entry and return */
  +            return DECLINED;
           }
           mobj->type = CACHE_TYPE_HEAP;
           h->cache_obj->count = 0;
  @@ -649,12 +668,17 @@
           apr_size_t len;
   
           if (APR_BUCKET_IS_EOS(e)) {
  +            mobj->cleanup = 0;
  +            mobj->refcount--;    /* Count should be 0 now */
  +            apr_pool_cleanup_kill(r->pool, h->cache_obj, decrement_refcount);
  +
  +            /* Open for business */
               h->cache_obj->complete = 1;
               break;
           }
           rv = apr_bucket_read(e, &s, &len, eblock);
           if (rv != APR_SUCCESS) {
  -            /* Big problem!  Cleanup cache entry and return */
  +            return DECLINED;
           }
           /* XXX Check for overflow */
           if (len ) {
  
  
  

Mime
View raw message