HipByte / Flow

Cross-platform libraries for RubyMotion
BSD 2-Clause "Simplified" License
141 stars 29 forks source link

Memory leak? #75

Closed amirrajan closed 7 years ago

amirrajan commented 7 years ago

Pasted from here.

I have been looking at the 'c' source code for creating a node in the Flow gem and have noticed what I think may be a memory leak. Here is the code for creating a new node:

static VALUE
node_alloc(VALUE rcv, SEL sel)
{
    struct ruby_css_node *node =
    (struct ruby_css_node *)malloc(sizeof(struct ruby_css_node));
    assert(node != NULL);
    node->parent = Qnil;
    node->node = new_css_node();
    assert(node->node != NULL);
    node->node->context = node;
    node->node->get_child = node_get_child;
    node->node->is_dirty = node_is_dirty;
    node->node->measure = NULL;
    node->children = rb_retain(rb_ary_new());
    node->dirty = true;
    node->obj = rb_weak(rb_class_wrap_new(node, rcv));
    return node->obj;
}

A node is created by using 'malloc' to allocate memory for the node structure which is then later wrapped in a ruby class and then a weak reference is created for it. This is all fine and I believe because of the wrapping the memory fro the structure will be released accordingly.

However, one of the fields in the node data structure called 'node' is created with the c function 'new_css_node()'

Here is the code for that function:

css_node_t *new_css_node() {
css_node_t *node = (css_node_t *)calloc(1, sizeof(*node));
       init_css_node(node);
  return node;
}

Notice there is a call to 'calloc' to allocate memory. However, since this is not wrapped in a ruby class then this memory is never released. Actually in the 'c' source there is a function to release this allocated memory but it never called. Here is that code:

void free_css_node(css_node_t *node) {
  free(node);
}

Can anyone confirm my findings or explain to me why this is not a memory leak?

lrz commented 7 years ago

free_css_node() is indeed never called but it doesn’t matter as the runtime will free() the internal memory of these objects upon finalisation. There is no memory leak :-)

On 11 May 2017, at 6:43 AM, Amir Rajan notifications@github.com wrote:

I have been looking at the 'c' source code for creating a node in the Flow gem and have noticed what I think may be a memory leak. Here is the code for creating a new node:

static VALUE node_alloc(VALUE rcv, SEL sel) { struct ruby_css_node node = (struct ruby_css_node )malloc(sizeof(struct ruby_css_node)); assert(node != NULL); node->parent = Qnil; node->node = new_css_node(); assert(node->node != NULL); node->node->context = node; node->node->get_child = node_get_child; node->node->is_dirty = node_is_dirty; node->node->measure = NULL; node->children = rb_retain(rb_ary_new()); node->dirty = true; node->obj = rb_weak(rb_class_wrap_new(node, rcv)); return node->obj; } A node is created by using 'malloc' to allocate memory for the node structure which is then later wrapped in a ruby class and then a weak reference is created for it. This is all fine and I believe because of the wrapping the memory fro the structure will be released accordingly.

However, one of the fields in the node data structure called 'node' is created with the c function 'new_css_node()'

Here is the code for that function:

css_node_t new_css_node() { css_node_t node = (css_node_t )calloc(1, sizeof(node)); init_css_node(node); return node; } Notice there is a call to 'calloc' to allocate memory. However, since this is not wrapped in a ruby class then this memory is never released. Actually in the 'c' source there is a function to release this allocated memory but it never called. Here is that code:

void free_css_node(css_node_t *node) { free(node); } Can anyone confirm my findings or explain to me why this is not a memory leak?

Thanks

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/HipByte/Flow/issues/75, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAiN5HJ5Rblj2V00f_vz4qlO08ds19Oks5r4xBlgaJpZM4NYBD5.

brucemontegani commented 7 years ago

Ok. Makes sense I guess. So what would happen if I created a data structure on the heap that was stored inside the css_node_t structure that represents the parent of the node. Also what if I wanted to create a list of nodes data structure on the heap stored inside the css_node_t structure that represents a list of children nodes. Would the memory of these data structures also be freed upon finalization?