aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPatrik Nyblom <[email protected]>2013-01-23 15:58:11 +0100
committerPatrik Nyblom <[email protected]>2013-01-23 15:58:39 +0100
commitdb4c3dbd2202ea27a91e97068f09e914522a042b (patch)
tree4f8a0ad77c8b12b6e7eabb9059e8d943b9765fd1
parentb7bdbc63af6c01db7cf8de667ef773c2b1d9b2c2 (diff)
parenta2ed83ab92c6267f5d5d459e69347c7db7e9ab48 (diff)
downloadotp-db4c3dbd2202ea27a91e97068f09e914522a042b.tar.gz
otp-db4c3dbd2202ea27a91e97068f09e914522a042b.tar.bz2
otp-db4c3dbd2202ea27a91e97068f09e914522a042b.zip
Merge branch 'pan/fdmanana/fix_efile_drv_crash'
* pan/fdmanana/fix_efile_drv_crash: Fix efile_drv crash when using async thread pool Alternative solution to the efile_drv crash on exit OTP-10748
-rw-r--r--erts/emulator/drivers/common/efile_drv.c93
-rw-r--r--lib/kernel/test/file_SUITE.erl57
2 files changed, 123 insertions, 27 deletions
diff --git a/erts/emulator/drivers/common/efile_drv.c b/erts/emulator/drivers/common/efile_drv.c
index 25b02db2c9..186af03eff 100644
--- a/erts/emulator/drivers/common/efile_drv.c
+++ b/erts/emulator/drivers/common/efile_drv.c
@@ -57,7 +57,7 @@
#define FILE_FADVISE 31
#define FILE_SENDFILE 32
#define FILE_FALLOCATE 33
-
+#define FILE_CLOSE_ON_PORT_EXIT 34
/* Return codes */
#define FILE_RESP_OK 0
@@ -178,6 +178,7 @@ dt_private *get_dt_private(int);
#define MUTEX_LOCK(m) do { IF_THRDS { TRACE_DRIVER; driver_pdl_lock(m); } } while (0)
#define MUTEX_UNLOCK(m) do { IF_THRDS { TRACE_DRIVER; driver_pdl_unlock(m); } } while (0)
#else
+#define IF_THRDS if (0)
#define MUTEX_INIT(m, p)
#define MUTEX_LOCK(m)
#define MUTEX_UNLOCK(m)
@@ -429,6 +430,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,34 +810,25 @@ 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);
+ 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);
}
-/*********************************************************************
- * Driver entry point -> stop
- */
-static void
-file_stop(ErlDrvData e)
-{
- file_descriptor* desc = (file_descriptor*)e;
-
- TRACE_C('p');
-
- if (desc->fd != FILE_FD_INVALID) {
- do_close(desc->flags, desc->fd);
- desc->fd = FILE_FD_INVALID;
- desc->flags = 0;
- }
- if (desc->read_binp) {
- driver_free_binary(desc->read_binp);
- }
- EF_FREE(desc);
-}
-
/*
* Sends back an error reply to Erlang.
@@ -2242,6 +2235,47 @@ static int lseek_flush_read(file_descriptor *desc, int *errp
}
+/*********************************************************************
+ * Driver entry point -> stop
+ * The close has to be scheduled on async thread, so that currently active
+ * async operation does not suddenly have the ground disappearing under their feet...
+ */
+static void
+file_stop(ErlDrvData e)
+{
+ file_descriptor* desc = (file_descriptor*)e;
+
+ TRACE_C('p');
+
+ IF_THRDS {
+ flush_read(desc);
+ if (desc->fd != FILE_FD_INVALID) {
+ struct t_data *d = EF_SAFE_ALLOC(sizeof(struct t_data));
+ d->command = FILE_CLOSE_ON_PORT_EXIT;
+ d->reply = !0;
+ d->fd = desc->fd;
+ d->flags = desc->flags;
+ 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;
+ cq_execute(desc);
+ }
+ } else {
+ if (desc->fd != FILE_FD_INVALID) {
+ do_close(desc->flags, desc->fd);
+ desc->fd = FILE_FD_INVALID;
+ desc->flags = 0;
+ }
+ if (desc->read_binp) {
+ driver_free_binary(desc->read_binp);
+ }
+ EF_FREE(desc);
+ }
+}
/*********************************************************************
* Driver entry point -> ready_async
@@ -2465,7 +2499,6 @@ file_async_ready(ErlDrvData e, ErlDrvThreadData data)
}
free_readdir(data);
break;
- /* See file_stop */
case FILE_CLOSE:
if (d->reply) {
TRACE_C('K');
@@ -2525,6 +2558,15 @@ file_async_ready(ErlDrvData e, ErlDrvThreadData data)
}
break;
#endif
+ case FILE_CLOSE_ON_PORT_EXIT:
+ /* See file_stop. However this is never invoked after the port is killed. */
+ free_data(data);
+ EF_FREE(desc);
+ desc = NULL;
+ /* This is it for this port, so just send dtrace and return, avoid doing anything to the freed data */
+ DTRACE6(efile_drv_return, sched_i1, sched_i2, sched_utag,
+ command, result_ok, posix_errno);
+ return;
default:
abort();
}
@@ -2535,6 +2577,7 @@ file_async_ready(ErlDrvData e, ErlDrvThreadData data)
driver_set_timer(desc->port, desc->write_delay);
}
cq_execute(desc);
+
}
diff --git a/lib/kernel/test/file_SUITE.erl b/lib/kernel/test/file_SUITE.erl
index 914f0d6127..f34341f561 100644
--- a/lib/kernel/test/file_SUITE.erl
+++ b/lib/kernel/test/file_SUITE.erl
@@ -60,7 +60,8 @@
-export([ read_not_really_compressed/1,
read_compressed_cooked/1, read_compressed_cooked_binary/1,
read_cooked_tar_problem/1,
- write_compressed/1, compress_errors/1, catenated_gzips/1]).
+ write_compressed/1, compress_errors/1, catenated_gzips/1,
+ compress_async_crash/1]).
-export([ make_link/1, read_link_info_for_non_link/1, symlinks/1]).
@@ -135,7 +136,8 @@ groups() ->
{compression, [],
[read_compressed_cooked, read_compressed_cooked_binary,
read_cooked_tar_problem, read_not_really_compressed,
- write_compressed, compress_errors, catenated_gzips]},
+ write_compressed, compress_errors, catenated_gzips,
+ compress_async_crash]},
{links, [],
[make_link, read_link_info_for_non_link, symlinks]}].
@@ -2312,6 +2314,57 @@ compress_errors(Config) when is_list(Config) ->
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+compress_async_crash(suite) -> [];
+compress_async_crash(doc) -> [];
+compress_async_crash(Config) when is_list(Config) ->
+ ?line DataDir = ?config(data_dir, Config),
+ ?line Path = filename:join(DataDir, "test.gz"),
+ ExpectedData = <<"qwerty">>,
+
+ ?line _ = ?FILE_MODULE:delete(Path),
+ ?line {ok, Fd} = ?FILE_MODULE:open(Path, [write, binary, compressed]),
+ ?line ok = ?FILE_MODULE:write(Fd, ExpectedData),
+ ?line ok = ?FILE_MODULE:close(Fd),
+
+ % Test that when using async thread pool, the emulator doesn't crash
+ % when the efile port driver is stopped while a compressed file operation
+ % is in progress (being carried by an async thread).
+ ?line ok = compress_async_crash_loop(10000, Path, ExpectedData),
+ ?line ok = ?FILE_MODULE:delete(Path),
+ ok.
+
+compress_async_crash_loop(0, _Path, _ExpectedData) ->
+ ok;
+compress_async_crash_loop(N, Path, ExpectedData) ->
+ Parent = self(),
+ {Pid, Ref} = spawn_monitor(
+ fun() ->
+ ?line {ok, Fd} = ?FILE_MODULE:open(
+ Path, [read, compressed, raw, binary]),
+ Len = byte_size(ExpectedData),
+ Parent ! {self(), continue},
+ ?line {ok, ExpectedData} = ?FILE_MODULE:read(Fd, Len),
+ ?line ok = ?FILE_MODULE:close(Fd),
+ receive foobar -> ok end
+ end),
+ receive
+ {Pid, continue} ->
+ exit(Pid, shutdown),
+ receive
+ {'DOWN', Ref, _, _, Reason} ->
+ ?line shutdown = Reason
+ end;
+ {'DOWN', Ref, _, _, Reason2} ->
+ test_server:fail({worker_exited, Reason2})
+ after 60000 ->
+ exit(Pid, shutdown),
+ erlang:demonitor(Ref, [flush]),
+ test_server:fail(worker_timeout)
+ end,
+ compress_async_crash_loop(N - 1, Path, ExpectedData).
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
altname(doc) ->
"Test the file:altname/1 function";
altname(suite) ->