From 4cdca97d6f4f871d4281a79c3cb3141b7c9740d2 Mon Sep 17 00:00:00 2001 From: Nick Reynolds Date: Mon, 25 Feb 2019 17:49:49 +0100 Subject: [PATCH] Safer NIFs and revised bindings API See the additions to CHANGELOG.md for details. --- CHANGELOG.md | 29 ++++ c_src/termbox_bindings.c | 199 ++++++++++++++++++++----- examples/event_viewer.exs | 5 +- examples/hello_world.exs | 1 + lib/ex_termbox/bindings.ex | 130 ++++++++++++---- lib/ex_termbox/event_manager.ex | 35 ++++- test/ex_termbox/event_manager_test.exs | 10 +- test/integration/bindings_test.exs | 80 ++++++++++ test/integration/examples_test.exs | 13 -- 9 files changed, 415 insertions(+), 87 deletions(-) create mode 100644 test/integration/bindings_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e6075..adbb840 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +The next release will include minor, but breaking changes to the termbox +bindings API in order to make working with the NIFs safer. + +Specifically, the termbox bindings have been updated to guard against undefined +behavior (e.g., double initialization, trying to shut it down when it hasn't +been initialized, getting the terminal height when it's not running, etc.). + +Similarly, the bindings now prevent polling for events in parallel (i.e., in +multiple NIF threads), which isn't safe. One way this might have happened before +is when an `EventManager` crashed and was restarted. + ### Changed +#### Breaking +- `Bindings.poll_event/1` will now return an error (`{:error, :already_polling}`) + if there's an existing polling thread. (See `Bindings.cancel_poll_event/0`). +- The `EventManager` server can now crash if it receives errors when attempting + to poll. It will attempt to cancel and restart polling once in order to + account for the gen_server being restarted. +- All `Bindings` functions (except `init/0`) can now return `{:error, :not_running}`. +- Changed return types for several `Bindings` functions to accomodate new errors: + - `Bindings.width/1` now returns `{:ok, width}` instead of `width`. + - `Bindings.height/1` now returns `{:ok, height}` instead of `height`. + - `Bindings.select_input_mode/1` now returns `{:ok, mode}` instead of `mode`. + - `Bindings.select_output_mode/1` now returns `{:ok, mode}` instead of `mode`. + +### Added +- `Bindings.cancel_poll_event/0` provides a way to cancel and later restart + polling (for example if the original process who called `poll_event/1` died.) + + ## [0.3.5] - 2019-02-21 ### Fixed - Event manager's default server `name`, which makes it possible to use the diff --git a/c_src/termbox_bindings.c b/c_src/termbox_bindings.c index 4185038..b252f90 100644 --- a/c_src/termbox_bindings.c +++ b/c_src/termbox_bindings.c @@ -1,11 +1,23 @@ +#include +#include + #include "erl_nif.h" #include "termbox.h" -#include + +static bool RUNNING = false; +static bool POLLING = false; +static bool STOP_POLLING = false; +ErlNifResourceType *POLL_STATE_RES_TYPE = NULL; +struct extb_poll_state *POLL_STATE = NULL; static ERL_NIF_TERM extb_ok(ErlNifEnv *env) { return enif_make_atom(env, "ok"); } +static ERL_NIF_TERM extb_ok_tuple(ErlNifEnv *env, ERL_NIF_TERM term) { + return enif_make_tuple2(env, enif_make_atom(env, "ok"), term); +} + static ERL_NIF_TERM extb_error(ErlNifEnv *env, const char *reason) { return enif_make_tuple2(env, enif_make_atom(env, "error"), enif_make_atom(env, reason)); @@ -13,39 +25,55 @@ static ERL_NIF_TERM extb_error(ErlNifEnv *env, const char *reason) { static ERL_NIF_TERM extb_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (RUNNING) { + return extb_error(env, "already_running"); + } + RUNNING = true; + int code = tb_init(); - if (code == 0) + if (code == 0) { return extb_ok(env); + } return enif_make_tuple2(env, enif_make_atom(env, "error"), enif_make_int(env, code)); } -static ERL_NIF_TERM extb_shutdown(ErlNifEnv *env, int argc, - const ERL_NIF_TERM argv[]) { - tb_shutdown(); - return extb_ok(env); -} - static ERL_NIF_TERM extb_width(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int32_t width = tb_width(); - return enif_make_int(env, width); + return extb_ok_tuple(env, enif_make_int(env, width)); } static ERL_NIF_TERM extb_height(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int32_t height = tb_height(); - return enif_make_int(env, height); + return extb_ok_tuple(env, enif_make_int(env, height)); } static ERL_NIF_TERM extb_clear(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + tb_clear(); return extb_ok(env); } static ERL_NIF_TERM extb_set_clear_attributes(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + unsigned int fg, bg; enif_get_uint(env, argv[0], &fg); enif_get_uint(env, argv[1], &bg); @@ -56,12 +84,20 @@ static ERL_NIF_TERM extb_set_clear_attributes(ErlNifEnv *env, int argc, static ERL_NIF_TERM extb_present(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + tb_present(); return extb_ok(env); } static ERL_NIF_TERM extb_set_cursor(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int x, y; enif_get_int(env, argv[0], &x); enif_get_int(env, argv[1], &y); @@ -72,6 +108,10 @@ static ERL_NIF_TERM extb_set_cursor(ErlNifEnv *env, int argc, static ERL_NIF_TERM extb_change_cell(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int x, y; unsigned int ch, fg, bg; @@ -87,18 +127,26 @@ static ERL_NIF_TERM extb_change_cell(ErlNifEnv *env, int argc, static ERL_NIF_TERM extb_select_input_mode(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int mode, result; enif_get_int(env, argv[0], &mode); result = tb_select_input_mode(mode); - return enif_make_int(env, result); + return extb_ok_tuple(env, enif_make_int(env, result)); } static ERL_NIF_TERM extb_select_output_mode(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + int mode, result; enif_get_int(env, argv[0], &mode); result = tb_select_output_mode(mode); - return enif_make_int(env, result); + return extb_ok_tuple(env, enif_make_int(env, result)); } // Passed to the created thread's function with information on where @@ -106,6 +154,7 @@ static ERL_NIF_TERM extb_select_output_mode(ErlNifEnv *env, int argc, // can later be joined. struct extb_poll_state { ErlNifTid thread_id; + bool thread_joined; ErlNifPid recipient_pid; }; @@ -113,45 +162,74 @@ void *extb_poll_event_run(void *arg) { struct extb_poll_state *state = (struct extb_poll_state *)arg; struct tb_event *event = enif_alloc(sizeof(struct tb_event)); - int event_type = tb_poll_event(event); + int poll_result = 0; + + while (!STOP_POLLING && poll_result == 0) { + poll_result = tb_peek_event(event, 50); + } - ErlNifEnv *env = enif_alloc_env(); - ERL_NIF_TERM result = enif_make_tuple2( - env, enif_make_atom(env, "event"), - enif_make_tuple8( - env, enif_make_uint(env, event->type), - enif_make_uint(env, event->mod), enif_make_uint(env, event->key), - enif_make_uint(env, event->ch), enif_make_int(env, event->w), - enif_make_int(env, event->h), enif_make_int(env, event->x), - enif_make_int(env, event->y))); + if (poll_result > 0) { + ErlNifEnv *env = enif_alloc_env(); + ERL_NIF_TERM result = enif_make_tuple2( + env, enif_make_atom(env, "event"), + enif_make_tuple8( + env, enif_make_uint(env, event->type), + enif_make_uint(env, event->mod), enif_make_uint(env, event->key), + enif_make_uint(env, event->ch), enif_make_int(env, event->w), + enif_make_int(env, event->h), enif_make_int(env, event->x), + enif_make_int(env, event->y))); + + // Send the event back to the configured recipient. + enif_send(NULL, &state->recipient_pid, env, result); + enif_free_env(env); + } - int sent = enif_send(NULL, &state->recipient_pid, env, result); - enif_free_env(env); enif_free(event); + + // Release the poll state resource for destruction and gc + // (BEAM will call `extb_poll_thread_cleanup`) + enif_release_resource(state); + + // Release the polling lock + POLLING = false; return NULL; }; -void extb_poll_thread_cleanup(ErlNifEnv *env, void *arg) { +void extb_join_poll_thread(struct extb_poll_state *state) { + if (!state->thread_joined) { + state->thread_joined = true; + enif_thread_join(state->thread_id, NULL); + } +} + +void extb_poll_thread_destructor(ErlNifEnv *env, void *arg) { struct extb_poll_state *state = (struct extb_poll_state *)arg; - void *resp; - enif_thread_join(state->thread_id, &resp); - enif_free(state); + extb_join_poll_thread(state); } static ERL_NIF_TERM extb_poll_event_async(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { + // Only one event polling thread may run at a time. + if (!RUNNING) { + return extb_error(env, "not_running"); + } + if (POLLING) { + return extb_error(env, "already_polling"); + } + STOP_POLLING = false; + POLLING = true; + // Create a resource as a handle for the thread - const char *resource_type_name = "extb-thread-handler"; - int flags = ERL_NIF_RT_CREATE | ERL_NIF_RT_TAKEOVER; - ErlNifResourceType *res_type = enif_open_resource_type( - env, NULL, resource_type_name, extb_poll_thread_cleanup, flags, NULL); struct extb_poll_state *poll_state = (struct extb_poll_state *)enif_alloc_resource( - res_type, sizeof(struct extb_poll_state)); + POLL_STATE_RES_TYPE, sizeof(struct extb_poll_state)); + poll_state->thread_joined = false; // Set the recipient pid to the pid arg enif_get_local_pid(env, argv[0], &poll_state->recipient_pid); + POLL_STATE = poll_state; + // Create a thread to perform the event polling int result = enif_thread_create("extb-event-poll", &poll_state->thread_id, extb_poll_event_run, poll_state, NULL); @@ -161,9 +239,53 @@ static ERL_NIF_TERM extb_poll_event_async(ErlNifEnv *env, int argc, enif_make_resource(env, poll_state)); } -static ErlNifFunc nif_funcs[] = { +static ERL_NIF_TERM extb_cancel_poll_event(ErlNifEnv *env, int argc, + const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + if (!POLLING) { + return extb_error(env, "not_polling"); + } + + if (POLL_STATE) { + STOP_POLLING = true; + extb_join_poll_thread(POLL_STATE); + POLL_STATE = NULL; + } + + return extb_ok(env); +} + +static ERL_NIF_TERM extb_shutdown(ErlNifEnv *env, int argc, + const ERL_NIF_TERM argv[]) { + if (!RUNNING) { + return extb_error(env, "not_running"); + } + RUNNING = false; + + if (POLL_STATE) { + STOP_POLLING = true; + extb_join_poll_thread(POLL_STATE); + POLL_STATE = NULL; + } + + tb_shutdown(); + return extb_ok(env); +} + +int extb_load(ErlNifEnv *env, void **priv_data, ERL_NIF_TERM load_info) { + // Create a resource type for the poll state + int flags = ERL_NIF_RT_CREATE | ERL_NIF_RT_TAKEOVER; + const char *resource_type_name = "extb-thread-handler"; + POLL_STATE_RES_TYPE = enif_open_resource_type( + env, NULL, resource_type_name, extb_poll_thread_destructor, flags, NULL); + + return 0; +} + +static ErlNifFunc funcs[] = { {"init", 0, extb_init}, - {"shutdown", 0, extb_shutdown}, {"width", 0, extb_width}, {"height", 0, extb_height}, {"clear", 0, extb_clear}, @@ -173,7 +295,8 @@ static ErlNifFunc nif_funcs[] = { {"change_cell", 5, extb_change_cell}, {"select_input_mode", 1, extb_select_input_mode}, {"select_output_mode", 1, extb_select_output_mode}, - {"poll_event", 1, extb_poll_event_async} -}; + {"poll_event", 1, extb_poll_event_async}, + {"cancel_poll_event", 0, extb_cancel_poll_event}, + {"shutdown", 0, extb_shutdown}}; -ERL_NIF_INIT(Elixir.ExTermbox.Bindings, nif_funcs, NULL, NULL, NULL, NULL) +ERL_NIF_INIT(Elixir.ExTermbox.Bindings, funcs, extb_load, NULL, NULL, NULL) diff --git a/examples/event_viewer.exs b/examples/event_viewer.exs index 7f2f4b5..64c29c2 100644 --- a/examples/event_viewer.exs +++ b/examples/event_viewer.exs @@ -8,8 +8,8 @@ defmodule EventViewer do alias ExTermbox.{Cell, Constants, EventManager, Event, Position} def run do - Termbox.init() - Termbox.select_input_mode(Constants.input_mode(:esc_with_mouse)) + :ok = Termbox.init() + {:ok, _} = Termbox.select_input_mode(Constants.input_mode(:esc_with_mouse)) {:ok, _pid} = EventManager.start_link() :ok = EventManager.subscribe(self()) @@ -21,6 +21,7 @@ defmodule EventViewer do def loop do receive do {:event, %Event{ch: ?q}} -> + :ok = EventManager.stop() :ok = Termbox.shutdown() {:event, %Event{} = event} -> diff --git a/examples/hello_world.exs b/examples/hello_world.exs index fb02557..02ca9e4 100644 --- a/examples/hello_world.exs +++ b/examples/hello_world.exs @@ -37,5 +37,6 @@ Termbox.present() # receive a 'q' key press, we'll shut down the application. receive do {:event, %Event{ch: ?q}} -> + :ok = EventManager.stop() :ok = Termbox.shutdown() end diff --git a/lib/ex_termbox/bindings.ex b/lib/ex_termbox/bindings.ex index 898421c..9a2b46a 100644 --- a/lib/ex_termbox/bindings.ex +++ b/lib/ex_termbox/bindings.ex @@ -46,10 +46,12 @@ defmodule ExTermbox.Bindings do Initializes the termbox library. Must be called before any other bindings are called. - Returns `:ok` on success. On error, returns a tuple `{:error, code}` - containing an integer representing a termbox error code. + Returns `:ok` on success and otherwise one of the following errors: + + * `{:error, :already_running} - the library was already initialized. + * `{:error, code}` - where code is an integer error code from termbox. """ - @spec init :: :ok | {:error, integer()} + @spec init :: :ok | {:error, integer() | :already_running} def init do error("NIF init/0 not loaded") end @@ -57,35 +59,50 @@ defmodule ExTermbox.Bindings do @doc """ Finalizes the termbox library. Should be called when the terminal application is exited, and before your program or OTP application stops. + + Returns `:ok` on success and otherwise one of the following errors: + + * `{:error, :not_running} - the library can not be shut down because it is not + initialized. + * `{:error, code}` - where `code` is an integer error code from termbox. """ - @spec shutdown :: :ok | {:error, integer()} + @spec shutdown :: :ok | {:error, integer() | :not_running} def shutdown do error("NIF shutdown/0 not loaded") end @doc """ - Returns the width of the terminal window in characters. Undefined before - `init/0` is called. + Returns `{:ok, width}` where `width` is the width of the terminal window in + characters. + + If termbox was not initialized, returns `{:error, :not_running}` (call + `init/0` first). """ - @spec width :: integer() + @spec width :: {:ok, integer()} | {:error, :not_running} def width do error("NIF width/0 not loaded") end @doc """ - Returns the height of the terminal window in characters. Undefined before - `init/0` is called. + Returns `{:ok, height}` where `height` is the height of the terminal window in + characters. + + If termbox was not initialized, returns `{:error, :not_running}` (call + `init/0` first). """ - @spec height :: integer() + @spec height :: {:ok, integer()} | {:error, :not_running} def height do error("NIF height/0 not loaded") end @doc """ - Clears the internal back buffer, setting the foreground and background to - the defaults, or those specified by `set_clear_attributes/2`. + Clears the internal back buffer, setting the foreground and background to the + defaults, or those specified by `set_clear_attributes/2`. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec clear :: :ok + @spec clear :: :ok | {:error, :not_running} def clear do error("NIF clear/0 not loaded") end @@ -93,25 +110,36 @@ defmodule ExTermbox.Bindings do @doc """ Sets the default foreground and background colors used when `clear/0` is called. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec set_clear_attributes(Constants.color(), Constants.color()) :: :ok + @spec set_clear_attributes(Constants.color(), Constants.color()) :: + :ok | {:error, :not_running} def set_clear_attributes(_fg, _bg) do error("NIF set_clear_attributes/2 not loaded") end @doc """ Synchronizes the internal back buffer and the terminal. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec present :: :ok + @spec present :: :ok | {:error, :not_running} def present do error("NIF present/0 not loaded") end @doc """ - Sets the position of the cursor to the coordinates `(x, y)`, or hide the cursor - by passing `ExTermbox.Constants.hide_cursor/0` for both x and y. + Sets the position of the cursor to the coordinates `(x, y)`, or hide the + cursor by passing `ExTermbox.Constants.hide_cursor/0` for both x and y. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec set_cursor(non_neg_integer(), non_neg_integer()) :: :ok + @spec set_cursor(non_neg_integer(), non_neg_integer()) :: + :ok | {:error, :not_running} def set_cursor(_x, _y) do error("NIF set_cursor/2 not loaded") end @@ -119,8 +147,11 @@ defmodule ExTermbox.Bindings do @doc """ Puts a cell in the internal back buffer at the cell's position. Note that this is implemented in terms of `change_cell/5`. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec put_cell(Cell.t()) :: :ok + @spec put_cell(Cell.t()) :: :ok | {:error, :not_running} def put_cell(%Cell{position: %Position{x: x, y: y}, ch: ch, fg: fg, bg: bg}) do change_cell(x, y, ch, fg, bg) end @@ -129,6 +160,9 @@ defmodule ExTermbox.Bindings do Changes the attributes of the cell at the specified position in the internal back buffer. Prefer using `put_cell/1`, which supports passing an `ExTermbox.Cell` struct. + + Returns `:ok` if successful. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ @spec change_cell( non_neg_integer(), @@ -136,7 +170,7 @@ defmodule ExTermbox.Bindings do non_neg_integer(), Constants.color(), Constants.color() - ) :: :ok + ) :: :ok | {:error, :not_running} def change_cell(_x, _y, _ch, _fg, _bg) do error("NIF change_cell/5 not loaded") end @@ -146,9 +180,12 @@ defmodule ExTermbox.Bindings do See the [termbox source](https://github.com/nsf/termbox/blob/master/src/termbox.h) for additional documentation. - Returns an integer representing the input mode. + Returns `{:ok, input_mode}` when successful, where `input_mode` is an integer + representing the current mode. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec select_input_mode(Constants.input_mode()) :: integer() + @spec select_input_mode(Constants.input_mode()) :: + {:ok, integer()} | {:error, :not_running} def select_input_mode(_mode) do error("NIF select_input_mode/1 not loaded") end @@ -158,9 +195,12 @@ defmodule ExTermbox.Bindings do See the [termbox source](https://github.com/nsf/termbox/blob/master/src/termbox.h) for additional documentation. - Returns an integer representing the output mode. + Returns `{:ok, output_mode}` when successful, where `output_mode` is an + integer representing the current mode. If termbox was not initialized, returns + `{:error, :not_running}` (call `init/0` first). """ - @spec select_output_mode(Constants.output_mode()) :: integer() + @spec select_output_mode(Constants.output_mode()) :: + {:ok, integer()} | {:error, :not_running} def select_output_mode(_mode) do error("NIF select_output_mode/1 not loaded") end @@ -176,17 +216,49 @@ defmodule ExTermbox.Bindings do scheduler. Note that the `ExTermbox.EventManager` is an abstraction over this function - that listens continuously for events and supports multiple subscriptions. + that listens continuously for events and supports multiple subscriptions. It's + recommended to use that abstraction and not to call this directly. + + If successful, returns `{:ok, resource}`, where `resource` is an Erlang + resource object representing a handle for the poll thread. + + ### Timeouts + + You might notice that there's no NIF binding for `tb_peek_event` (which + accepts a timeout). This function can also be used to achieve a timeout: + + {:ok, _resource} = Bindings.poll_event(self()) + + receive do + {:event, event} -> + # handle the event... + after + 1_000 -> + :ok = Bindings.cancel_poll_event(self()) + # do something else... + end - Returns a resource representing a handle for the poll thread. """ - @spec poll_event(pid()) :: reference() - def poll_event(pid) when is_pid(pid) do + @spec poll_event(pid()) :: + {:ok, reference()} | {:error, :not_running | :already_polling} + def poll_event(recipient_pid) when is_pid(recipient_pid) do error("NIF poll_event/1 not loaded") end @doc """ - *Not yet implemented.* For most cases, `poll_event/1` should be sufficient. + Cancels a previous call to `poll_event/1` and blocks until polling has + stopped. The polling loop checks every 10 ms for a stop condition, so calls + can take up to 10 ms to return. + + This can be useful, for example, if the `poll_event` recipient process dies + and the polling needs to be restarted by another process. + """ + @spec cancel_poll_event() :: :ok | {:error, :not_running | :not_polling} + def cancel_poll_event do + error("NIF cancel_poll_event/1 not loaded") + end + + @doc """ """ def peek_event(pid, _timeout) when is_pid(pid) do error("NIF peek_event/1 not loaded") diff --git a/lib/ex_termbox/event_manager.ex b/lib/ex_termbox/event_manager.ex index af9be0a..620d732 100644 --- a/lib/ex_termbox/event_manager.ex +++ b/lib/ex_termbox/event_manager.ex @@ -65,10 +65,16 @@ defmodule ExTermbox.EventManager do GenServer.call(event_manager_server, {:subscribe, subscriber_pid}) end + def stop(event_manager_server \\ __MODULE__) do + GenServer.stop(event_manager_server) + end + # Server Callbacks @impl true def init(bindings) do + _ = Process.flag(:trap_exit, true) + {:ok, %{ bindings: bindings, @@ -80,7 +86,7 @@ defmodule ExTermbox.EventManager do @impl true def handle_call({:subscribe, pid}, _from, state) do if state.status == :ready do - start_polling(state.bindings) + :ok = start_polling(state.bindings) end {:reply, :ok, @@ -98,10 +104,10 @@ defmodule ExTermbox.EventManager do def handle_info({:event, %Event{} = event}, state) do # Notify subscribers of the event - notify(state.recipients, event) + :ok = notify(state.recipients, event) # Start polling for the next event - start_polling(state.bindings) + :ok = start_polling(state.bindings) {:noreply, state} end @@ -110,14 +116,35 @@ defmodule ExTermbox.EventManager do {:noreply, state} end + @impl true + def terminate(_reason, state) do + # Try to cancel polling for events to leave the system in a clean state. If + # this fails or `terminate/2` isn't called, it will have to be done later. + _ = state.bindings.cancel_poll_event() + :ok + end + defp start_polling(bindings) do - bindings.poll_event(self()) + case bindings.poll_event(self()) do + {:ok, _resource} -> + :ok + + {:error, :already_polling} -> + with :ok <- bindings.cancel_poll_event(), + {:ok, _resource} <- bindings.poll_event(self()), + do: :ok + + {:error, unhandled_error} -> + {:error, unhandled_error} + end end defp notify(recipients, event) do for pid <- recipients do send(pid, {:event, event}) end + + :ok end defp unpack_event({type, mod, key, ch, w, h, x, y}) do diff --git a/test/ex_termbox/event_manager_test.exs b/test/ex_termbox/event_manager_test.exs index 92e8784..436a5d9 100644 --- a/test/ex_termbox/event_manager_test.exs +++ b/test/ex_termbox/event_manager_test.exs @@ -8,7 +8,15 @@ defmodule ExTermbox.EventManagerTest do def start_link(_), do: Agent.start_link(fn -> [] end, name: __MODULE__) - def poll_event(pid), do: track({:poll_event, pid}) + def poll_event(pid) do + track({:poll_event, pid}) + {:ok, nil} + end + + def cancel_poll_event do + track(:cancel_poll_event) + :ok + end def calls, do: Agent.get(__MODULE__, & &1) diff --git a/test/integration/bindings_test.exs b/test/integration/bindings_test.exs new file mode 100644 index 0000000..eae7136 --- /dev/null +++ b/test/integration/bindings_test.exs @@ -0,0 +1,80 @@ +defmodule ExTermbox.Integration.BindingsTest do + use ExUnit.Case, async: false + + alias ExTermbox.Bindings + + setup do + on_exit(fn -> + _ = Bindings.cancel_poll_event() + _ = Bindings.shutdown() + end) + + :ok + end + + describe "init/0" do + @tag :integration + test "returns an error if already running" do + assert :ok = Bindings.init() + assert {:error, :already_running} = Bindings.init() + + assert :ok = Bindings.shutdown() + assert :ok = Bindings.init() + end + end + + describe "shutdown/0" do + @tag :integration + test "returns :ok if sucessfully shutdown" do + :ok = Bindings.init() + + assert :ok = Bindings.shutdown() + end + + @tag :integration + test "returns an error if not running" do + assert {:error, :not_running} = Bindings.shutdown() + end + end + + describe "poll_event/1" do + @tag :integration + test "returns an error if not running" do + assert {:error, :not_running} = Bindings.poll_event(self()) + end + + @tag :integration + test "returns an error if already polling" do + :ok = Bindings.init() + + assert {:ok, _resource} = Bindings.poll_event(self()) + assert {:error, :already_polling} = Bindings.poll_event(self()) + + :ok = Bindings.cancel_poll_event() + + assert {:ok, _resource} = Bindings.poll_event(self()) + end + end + + describe "cancel_poll_event/0" do + @tag :integration + test "cancels the previous poll event call" do + :ok = Bindings.init() + {:ok, _resource} = Bindings.poll_event(self()) + + assert :ok = Bindings.cancel_poll_event() + end + + @tag :integration + test "returns an error if not running" do + assert {:error, :not_running} = Bindings.poll_event(self()) + end + + @tag :integration + test "returns an error if not polling" do + :ok = Bindings.init() + + {:error, :not_polling} = Bindings.cancel_poll_event() + end + end +end diff --git a/test/integration/examples_test.exs b/test/integration/examples_test.exs index 8611d40..2c292ea 100644 --- a/test/integration/examples_test.exs +++ b/test/integration/examples_test.exs @@ -13,19 +13,6 @@ defmodule ExTermbox.Integration.ExamplesTest do @examples_root Path.join(Path.dirname(__ENV__.file), "../../examples") @examples Path.wildcard("#{@examples_root}/*.exs") - setup do - on_exit(fn -> - event_mgr = event_manager() - - if is_pid(event_mgr) do - :ok = GenServer.stop(event_mgr, :normal) - end - end) - - :ok - end - - @tag :integration test "at least one example is found" do assert [_ | _] = @examples end