search

datamapper / do

DataObjects
147 stars 74 forks source link

memory leak fix - rb_fd_term was missing #91

Closed dgrunberg closed 8 years ago

dgrunberg commented 8 years ago

Our production cloud application was leaking memory (about 240 MB per day). It was determined to be in datamapper - 160 bytes per MySQL read access. The problem is in do_mysql.c, where, if HAVE_RB_THREAD_FD_SELECT is defined (a newer version of Ruby, I believe), rb_fd_init is called (which ALLOCs memory), and rb_fd_term is never called. If HAVE_RB_THREAD_FD_SELECT is not defined, then the rb_fd_init does not ALLOC memory, and rb_fd_term is defined as a no-op, so there is no harm in calling rb_fd_term in that case. I have added the call to rb_fd_term in the correct place. The two attached plots show the memory usage of our application over 7 days, the only difference being the with and without the patch. We restarted our application half-way through the plot because we were about to run out of memory. The patch passes all 163 'rake spec' tests.

mem1 mem2

dbussink commented 8 years ago

Thanks for this fix, good find! I'm going to apply these changes but also fix do_postgres which has a similar issue. There are also some failure paths in the while loop that would need the same cleanup so adding that there too.

dbussink commented 8 years ago

Pull request is superseded by #93