Anonymous avatar Anonymous committed 5f2ef84

usnic: fix several memory leaks

- some free lists simply were not being OBJ_DESTRUCTed, so they never
freed their internal memory

- channel->recv_segs.ctx was being assigned in a way that got clobbered
by ompi_free_list_init_new, so the cleanup code that relied on it
being set never ran

- numerous other ".ctx" assignments were similarly ineffectual and were
not being consumed, so I deleted them

Reviewed-by: Jeff Squyres <jsquyres@cisco.com>;

Comments (0)

Files changed (1)

ompi/mca/btl/usnic/btl_usnic_module.c

      * (i.e., so that their super/opal_list_item_t member isn't still
      * in use) 
      * This call to usnic_finalize is actually an implicit ACK of
-     * every packet we have ever sent, so call handle_ack to do all 
-     * the ACK processing and release all the data that needs releasing.
+     * every packet we have ever sent, so call handle_ack (via
+     * flush_endpoint) to do all the ACK processing and release all the
+     * data that needs releasing.
      *
      * If we don't care about releasing data, we could just set:
      *   endpoint->endpoint_ack_seq_rcvd =
     }
     OBJ_DESTRUCT(&(module->all_endpoints));
 
+    /* _flush_endpoint should have emptied this list */
+    assert(opal_list_is_empty(&(module->pending_resend_segs)));
+    OBJ_DESTRUCT(&module->pending_resend_segs);
+
     /* Similarly, empty the endpoints_that_need_acks list so that
        endpoints don't still have an endpoint_ack_li item still in
        use */
     }
     OBJ_DESTRUCT(&module->all_procs);
 
+    for (i = module->first_pool; i <= module->last_pool; ++i) {
+        OBJ_DESTRUCT(&module->module_recv_buffers[i]);
+    }
+    free(module->module_recv_buffers);
+
     OBJ_DESTRUCT(&module->ack_segs);
     OBJ_DESTRUCT(&module->endpoints_with_sends);
     OBJ_DESTRUCT(&module->small_send_frags);
     OBJ_DESTRUCT(&module->large_send_frags);
     OBJ_DESTRUCT(&module->put_dest_frags);
+    OBJ_DESTRUCT(&module->chunk_segs);
     OBJ_DESTRUCT(&module->senders);
     mca_mpool_base_module_destroy(module->super.btl_mpool);
 
 {
     /* gets set right after constructor called, lets us know recv_segs
      * have been constructed
-    */
+     */
     if (channel->recv_segs.ctx == module) {
         OBJ_DESTRUCT(&channel->recv_segs);
     }
      */
     segsize = (mtu + opal_cache_line_size - 1) & ~(opal_cache_line_size - 1);
     OBJ_CONSTRUCT(&channel->recv_segs, ompi_free_list_t);
-    channel->recv_segs.ctx = module;
     rc = ompi_free_list_init_new(&channel->recv_segs,
                                  sizeof(ompi_btl_usnic_recv_segment_t),
                                  opal_cache_line_size,
                                  rd_num,
                                  rd_num,
                                  module->super.btl_mpool);
+    channel->recv_segs.ctx = module; /* must come after ompi_free_list_init_new,
+                                        otherwise ctx gets clobbered */
     if (OMPI_SUCCESS != rc) {
         goto error;
     }
         ~(opal_cache_line_size - 1);
 
     /* Send frags freelists */
-    module->small_send_frags.ctx = module;
     OBJ_CONSTRUCT(&module->small_send_frags, ompi_free_list_t);
     rc = ompi_free_list_init_new(&module->small_send_frags,
                                  sizeof(ompi_btl_usnic_small_send_frag_t),
                                  module->super.btl_mpool);
     assert(OMPI_SUCCESS == rc);
 
-    module->large_send_frags.ctx = module;
     OBJ_CONSTRUCT(&module->large_send_frags, ompi_free_list_t);
     rc = ompi_free_list_init_new(&module->large_send_frags,
                                  sizeof(ompi_btl_usnic_large_send_frag_t),
                                  NULL);
     assert(OMPI_SUCCESS == rc);
 
-    module->put_dest_frags.ctx = module;
     OBJ_CONSTRUCT(&module->put_dest_frags, ompi_free_list_t);
     rc = ompi_free_list_init_new(&module->put_dest_frags,
                                  sizeof(ompi_btl_usnic_put_dest_frag_t),
     assert(OMPI_SUCCESS == rc);
 
     /* list of segments to use for sending */
-    module->chunk_segs.ctx = module;
     OBJ_CONSTRUCT(&module->chunk_segs, ompi_free_list_t);
     rc = ompi_free_list_init_new(&module->chunk_segs,
                                  sizeof(ompi_btl_usnic_chunk_segment_t),
     assert(OMPI_SUCCESS == rc);
 
     /* ACK segments freelist */
-    module->ack_segs.ctx = module;
     ack_segment_len = (sizeof(ompi_btl_usnic_btl_header_t) +
             opal_cache_line_size - 1) & ~(opal_cache_line_size - 1);
     OBJ_CONSTRUCT(&module->ack_segs, ompi_free_list_t);
 
     /*
      * Initialize pools of large recv buffers
+     *
+     * NOTE: (last_pool < first_pool) is _not_ erroneous; recv buffer pools
+     * simply won't be used in that case.
      */
-    module->first_pool = 16;
+    module->first_pool = 16; /* 64 kiB */
     module->last_pool = fls(module->super.btl_eager_limit-1);
     module->module_recv_buffers = calloc(module->last_pool+1,
             sizeof(ompi_free_list_t));
     assert(module->module_recv_buffers != NULL);
     for (i=module->first_pool; i<=module->last_pool; ++i) {
-        module->module_recv_buffers[i].ctx = module;
         OBJ_CONSTRUCT(&module->module_recv_buffers[i], ompi_free_list_t);
         rc = ompi_free_list_init_new(&module->module_recv_buffers[i],
                                      1 << i,
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.