Skip to content

Instantly share code, notes, and snippets.

@fdmanana
Created January 16, 2013 10:35
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save fdmanana/4546241 to your computer and use it in GitHub Desktop.
Save fdmanana/4546241 to your computer and use it in GitHub Desktop.
From b3895efe1cff61182dfe76fed25c00d6cb590a71 Mon Sep 17 00:00:00 2001
From: Filipe David Borba Manana <fdmanana@apache.org>
Date: Wed, 16 Jan 2013 10:18:36 +0000
Subject: [PATCH] Fix memory leak in efile_drv
When the port's stop callback (file_stop) is invoked, we
schedule a file close task (FILE_CLOSE_ON_PORT_EXIT) that
will be picked up by an async thread, to avoid blocking the
scheduler thread for too long. However, because after this
task is executed the port is not alive anymore, its callback
ready_async (file_async_ready) can no longer be invoked - and
it's this callback that releases the port's data (a
file_descriptor structure), causing a memory leak.
To avoid this leak, simply let FILE_CLOSE_ON_PORT_EXIT tasks
to point to the file_descriptor structure and have their free
function release the file_descriptor structure.
---
erts/emulator/drivers/common/efile_drv.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/erts/emulator/drivers/common/efile_drv.c b/erts/emulator/drivers/common/efile_drv.c
index 1586e29..79e2ad3 100644
--- a/erts/emulator/drivers/common/efile_drv.c
+++ b/erts/emulator/drivers/common/efile_drv.c
@@ -429,6 +429,7 @@ struct t_data
int level;
void (*invoke)(void *);
void (*free)(void *);
+ void *data_to_free; /* used by FILE_CLOSE_ON_PORT_EXIT only */
int again;
int reply;
#ifdef USE_VM_PROBES
@@ -808,12 +809,20 @@ static void free_data(void *data)
{
struct t_data *d = (struct t_data *) data;
- if (d->command == FILE_OPEN && d->is_fd_unused && d->fd != FILE_FD_INVALID) {
- do_close(d->flags, d->fd); /* This is OK to do in scheduler thread because there can be no async op
- ongoing for this fd here, as we exited during async open.
- Ideally, this close should happen in an async thread too, but that would
- require a substantial rewrite, as we are here because of a dead port and
- cannot schedule async jobs for that port any more... */
+ switch (d->command) {
+ case FILE_OPEN:
+ if (d->is_fd_unused && d->fd != FILE_FD_INVALID) {
+ /* This is OK to do in scheduler thread because there can be no async op
+ ongoing for this fd here, as we exited during async open.
+ Ideally, this close should happen in an async thread too, but that would
+ require a substantial rewrite, as we are here because of a dead port and
+ cannot schedule async jobs for that port any more... */
+ do_close(d->flags, d->fd);
+ }
+ break;
+ case FILE_CLOSE_ON_PORT_EXIT:
+ EF_FREE(d->data_to_free);
+ break;
}
EF_FREE(data);
@@ -2248,6 +2257,7 @@ file_stop(ErlDrvData e)
d->invoke = invoke_close;
d->free = free_data;
d->level = 2;
+ d->data_to_free = (void *) desc;
cq_enq(desc, d);
desc->fd = FILE_FD_INVALID;
desc->flags = 0;
@@ -2548,7 +2558,7 @@ file_async_ready(ErlDrvData e, ErlDrvThreadData data)
break;
#endif
case FILE_CLOSE_ON_PORT_EXIT:
- /* See file_stop */
+ /* See file_stop. However this is never invoked after the port is killed. */
free_data(data);
EF_FREE(desc);
desc = NULL;
--
1.7.9.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment