add safety check for missing file (#2)

* add safety check for missing file * set `index->val` to `nullptr` after deleting it * updated test for loading missing file --------- Co-authored-by: Cocoa <[email protected]>
elixir-nx · Jun 4, 2023 · 3165951 · 3165951
1 parent a447799
commit 3165951
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 6 deletions.
diff --git a/c_src/hnswlib_nif.cpp b/c_src/hnswlib_nif.cpp
@@ -59,6 +59,7 @@ static ERL_NIF_TERM hnswlib_index_new(ErlNifEnv *env, int argc, const ERL_NIF_TE
     } catch (std::runtime_error &err) {
         if (index->val) {
             delete index->val;
+            index->val = nullptr;
         }
         enif_release_resource(index);
         return erlang::nif::error(env, err.what());
@@ -229,15 +230,15 @@ static ERL_NIF_TERM hnswlib_index_load_index(ErlNifEnv *env, int argc, const ERL
         index->val->loadIndex(path, max_elements, allow_replace_deleted);
 
         ret = erlang::nif::ok(env, enif_make_resource(env, index));
-        enif_release_resource(index);
     } catch (std::runtime_error &err) {
         if (index->val) {
             delete index->val;
+            index->val = nullptr;
         }
-        enif_release_resource(index);
         ret = erlang::nif::error(env, err.what());
     }
     enif_rwlock_rwunlock(index->rwlock);
+    enif_release_resource(index);
 
     return ret;
 }
@@ -513,6 +514,7 @@ static ERL_NIF_TERM hnswlib_bfindex_new(ErlNifEnv *env, int argc, const ERL_NIF_
     } catch (std::runtime_error &err) {
         if (index->val) {
             delete index->val;
+            index->val = nullptr;
         }
         enif_release_resource(index);
         return erlang::nif::error(env, err.what());
@@ -686,15 +688,15 @@ static ERL_NIF_TERM hnswlib_bfindex_load_index(ErlNifEnv *env, int argc, const E
         index->val->loadIndex(path, max_elements);
 
         ret = erlang::nif::ok(env, enif_make_resource(env, index));
-        enif_release_resource(index);
     } catch (std::runtime_error &err) {
         if (index->val) {
             delete index->val;
+            index->val = nullptr;
         }
         ret = erlang::nif::error(env, err.what());
-        enif_release_resource(index);
     }
     enif_rwlock_runlock(index->rwlock);
+    enif_release_resource(index);
 
     return ret;
 }

diff --git a/lib/hnswlib_index.ex b/lib/hnswlib_index.ex
@@ -248,8 +248,6 @@ defmodule HNSWLib.Index do
     max_elements = Helper.get_keyword!(opts, :max_elements, :non_neg_integer, 0)
     allow_replace_deleted = Helper.get_keyword!(opts, :allow_replace_deleted, :boolean, false)
 
-    HNSWLib.Nif.index_load_index(space, dim, path, max_elements, allow_replace_deleted)
-
     with {:ok, ref} <-
            HNSWLib.Nif.index_load_index(space, dim, path, max_elements, allow_replace_deleted) do
       {:ok,

diff --git a/test/hnswlib_index_test.exs b/test/hnswlib_index_test.exs
@@ -619,6 +619,12 @@ defmodule HNSWLib.Index.Test do
     File.rm(save_to)
   end
 
+  test "HNSWLib.Index.load_index/3 with missing file" do
+    bad_filepath = "this/file/doesnt/exist"
+    refute File.exists?(bad_filepath)
+    assert {:error, "Cannot open file"} = HNSWLib.Index.load_index(:l2, 2, bad_filepath)
+  end
+
   test "HNSWLib.Index.mark_deleted/2" do
     space = :ip
     dim = 2