Opened 6 years ago

Closed 6 years ago

#772 closed defect (fixed)

Review commit 498ced18a4 for reference leaks

Reported by: Jakub Jermář Owned by: Jakub Jermář
Priority: major Milestone: 0.8.0
Component: helenos/unspecified Version: mainline
Keywords: Cc:
Blocker for: Depends on:
See also: #744

Description (last modified by Jakub Jermář)

Commit 498ced18a4 introduced a new reference counting API into uspace. The semantics of the API might differ from the semantics of the code it replaced and so there is a possibility of reference leaks analogous to #744.

For example, changes to devman look fishy and analogous to #744:

--- a/uspace/srv/devman/fun.c
+++ b/uspace/srv/devman/fun.c
@@ -59,7 +59,7 @@ fun_node_t *create_fun_node(void)
                return NULL;
 
        fun->state = FUN_INIT;
-       atomic_set(&fun->refcnt, 0);
+       refcount_init(&fun->refcnt);
        fibril_mutex_initialize(&fun->busy_lock);
        link_initialize(&fun->dev_functions);
        list_initialize(&fun->match_ids.ids);

So the caller of create_fun_node() already has a reference created by refcount_init(). But then at the two locations that call create_fun_node(), we see the reference count bumped. In devman_add_function, there is:

        fun_node_t *fun = create_fun_node();
        /* One reference for creation, one for us */
        fun_add_ref(fun);
        fun_add_ref(fun);

This does not look right wrt. the new semantics.

Change History (5)

comment:1 by Jakub Jermář, 6 years ago

Description: modified (diff)
Summary: Review commit 498ced18a4 for memory leaksReview commit 498ced18a4 for reference leaks

comment:2 by Jakub Jermář, 6 years ago

The devman create_fun_node() issue is addressed in #PR63.

comment:3 by Jakub Jermář, 6 years ago

There seems to be a similar issue in libusbhost with endpoints. See #PR64.

comment:5 by Jakub Jermář, 6 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.