refactor(api): provide a temporary copy solution for nvim_call_atomic

Make the copy_object() family accept an optional arena. More than
half of the callsites should be refactored to use an arena later
anyway.

17 files changed, 71 insertions, 64 deletions

diff --git a/src/nvim/api/autocmd.c b/src/nvim/api/autocmd.c
index 79ae7994f7..1cf0211f43 100644
--- a/src/nvim/api/autocmd.c
+++ b/src/nvim/api/autocmd.c
@@ -887,12 +887,12 @@ static bool check_autocmd_string_array(Array arr, char *k, Error *err)
 static bool unpack_string_or_array(Array *array, Object *v, char *k, bool required, Error *err)
 {
   if (v->type == kObjectTypeString) {
-    ADD(*array, copy_object(*v));
+    ADD(*array, copy_object(*v, NULL));
   } else if (v->type == kObjectTypeArray) {
     if (!check_autocmd_string_array(v->data.array, k, err)) {
       return false;
     }
-    *array = copy_array(v->data.array);
+    *array = copy_array(v->data.array, NULL);
   } else {
     if (required) {
       api_set_error(err,
diff --git a/src/nvim/api/extmark.c b/src/nvim/api/extmark.c
index 933aa85530..1d6eaa42b0 100644
--- a/src/nvim/api/extmark.c
+++ b/src/nvim/api/extmark.c
@@ -51,7 +51,7 @@ Integer nvim_create_namespace(String name)
   }
   id = next_namespace_id++;
   if (name.size > 0) {
-    String name_alloc = copy_string(name);
+    String name_alloc = copy_string(name, NULL);
     map_put(String, handle_T)(&namespace_ids, name_alloc, id);
   }
   return (Integer)id;
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index c466fc53e1..e35c58bf1b 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -618,6 +618,7 @@ void api_clear_error(Error *value)
   value->type = kErrorTypeNone;
 }
 
+/// @returns a shared value. caller must not modify it!
 Dictionary api_metadata(void)
 {
   static Dictionary metadata = ARRAY_DICT_INIT;
@@ -630,7 +631,7 @@ Dictionary api_metadata(void)
     init_type_metadata(&metadata);
   }
 
-  return copy_object(DICTIONARY_OBJ(metadata)).data.dictionary;
+  return metadata;
 }
 
 static void init_function_metadata(Dictionary *metadata)
@@ -715,36 +716,40 @@ static void init_type_metadata(Dictionary *metadata)
   PUT(*metadata, "types", DICTIONARY_OBJ(types));
 }
 
-String copy_string(String str)
+// all the copy_[object] functions allow arena=NULL,
+// then global allocations are used, and the resulting object
+// should be freed with an api_free_[object] function
+
+String copy_string(String str, Arena *arena)
 {
   if (str.data != NULL) {
-    return (String){ .data = xmemdupz(str.data, str.size), .size = str.size };
+    return (String){ .data = arena_memdupz(arena, str.data, str.size), .size = str.size };
   } else {
     return (String)STRING_INIT;
   }
 }
 
-Array copy_array(Array array)
+Array copy_array(Array array, Arena *arena)
 {
-  Array rv = ARRAY_DICT_INIT;
+  Array rv = arena_array(arena, array.size);
   for (size_t i = 0; i < array.size; i++) {
-    ADD(rv, copy_object(array.items[i]));
+    ADD(rv, copy_object(array.items[i], arena));
   }
   return rv;
 }
 
-Dictionary copy_dictionary(Dictionary dict)
+Dictionary copy_dictionary(Dictionary dict, Arena *arena)
 {
-  Dictionary rv = ARRAY_DICT_INIT;
+  Dictionary rv = arena_dict(arena, dict.size);
   for (size_t i = 0; i < dict.size; i++) {
     KeyValuePair item = dict.items[i];
-    PUT(rv, item.key.data, copy_object(item.value));
+    PUT_C(rv, copy_string(item.key, arena).data, copy_object(item.value, arena));
   }
   return rv;
 }
 
 /// Creates a deep clone of an object
-Object copy_object(Object obj)
+Object copy_object(Object obj, Arena *arena)
 {
   switch (obj.type) {
   case kObjectTypeBuffer:
@@ -757,13 +762,13 @@ Object copy_object(Object obj)
     return obj;
 
   case kObjectTypeString:
-    return STRING_OBJ(copy_string(obj.data.string));
+    return STRING_OBJ(copy_string(obj.data.string, arena));
 
   case kObjectTypeArray:
-    return ARRAY_OBJ(copy_array(obj.data.array));
+    return ARRAY_OBJ(copy_array(obj.data.array, arena));
 
   case kObjectTypeDictionary:
-    return DICTIONARY_OBJ(copy_dictionary(obj.data.dictionary));
+    return DICTIONARY_OBJ(copy_dictionary(obj.data.dictionary, arena));
 
   case kObjectTypeLuaRef:
     return LUAREF_OBJ(api_new_luaref(obj.data.luaref));
@@ -844,7 +849,7 @@ HlMessage parse_hl_msg(Array chunks, Error *err)
       goto free_exit;
     }
 
-    String str = copy_string(chunk.items[0].data.string);
+    String str = copy_string(chunk.items[0].data.string, NULL);
 
     int attr = 0;
     if (chunk.size == 2) {
diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index 6f7bfa244a..e34dcbdb46 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -965,7 +965,7 @@ static Array translate_contents(UI *ui, Array contents)
     } else {
       ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT));
     }
-    ADD(new_item, copy_object(item.items[1]));
+    ADD(new_item, copy_object(item.items[1], NULL));
     ADD(new_contents, ARRAY_OBJ(new_item));
   }
   return new_contents;
@@ -978,7 +978,7 @@ static Array translate_firstarg(UI *ui, Array args)
 
   ADD(new_args, ARRAY_OBJ(translate_contents(ui, contents)));
   for (size_t i = 1; i < args.size; i++) {
-    ADD(new_args, copy_object(args.items[i]));
+    ADD(new_args, copy_object(args.items[i], NULL));
   }
   return new_args;
 }
@@ -1024,7 +1024,7 @@ static void remote_ui_event(UI *ui, char *name, Array args)
         Array items = args.items[0].data.array;
         Array new_items = ARRAY_DICT_INIT;
         for (size_t i = 0; i < items.size; i++) {
-          ADD(new_items, copy_object(items.items[i].data.array.items[0]));
+          ADD(new_items, copy_object(items.items[i].data.array.items[0], NULL));
         }
         ADD_C(new_args, ARRAY_OBJ(new_items));
         push_call(ui, "wildmenu_show", new_args);
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index c333db1b37..7b70de7b12 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -1479,14 +1479,14 @@ void nvim_del_keymap(uint64_t channel_id, String mode, String lhs, Error *err)
 /// 1 is the |api-metadata| map (Dictionary).
 ///
 /// @returns 2-tuple [{channel-id}, {api-metadata}]
-Array nvim_get_api_info(uint64_t channel_id)
+Array nvim_get_api_info(uint64_t channel_id, Arena *arena)
   FUNC_API_SINCE(1) FUNC_API_FAST FUNC_API_REMOTE_ONLY
 {
-  Array rv = ARRAY_DICT_INIT;
+  Array rv = arena_array(arena, 2);
 
   assert(channel_id <= INT64_MAX);
-  ADD(rv, INTEGER_OBJ((int64_t)channel_id));
-  ADD(rv, DICTIONARY_OBJ(api_metadata()));
+  ADD_C(rv, INTEGER_OBJ((int64_t)channel_id));
+  ADD_C(rv, DICTIONARY_OBJ(api_metadata()));
 
   return rv;
 }
@@ -1545,9 +1545,9 @@ void nvim_set_client_info(uint64_t channel_id, String name, Dictionary version,
   FUNC_API_SINCE(4) FUNC_API_REMOTE_ONLY
 {
   Dictionary info = ARRAY_DICT_INIT;
-  PUT(info, "name", copy_object(STRING_OBJ(name)));
+  PUT(info, "name", copy_object(STRING_OBJ(name), NULL));
 
-  version = copy_dictionary(version);
+  version = copy_dictionary(version, NULL);
   bool has_major = false;
   for (size_t i = 0; i < version.size; i++) {
     if (strequal(version.items[i].key.data, "major")) {
@@ -1560,9 +1560,9 @@ void nvim_set_client_info(uint64_t channel_id, String name, Dictionary version,
   }
   PUT(info, "version", DICTIONARY_OBJ(version));
 
-  PUT(info, "type", copy_object(STRING_OBJ(type)));
-  PUT(info, "methods", DICTIONARY_OBJ(copy_dictionary(methods)));
-  PUT(info, "attributes", DICTIONARY_OBJ(copy_dictionary(attributes)));
+  PUT(info, "type", copy_object(STRING_OBJ(type), NULL));
+  PUT(info, "methods", DICTIONARY_OBJ(copy_dictionary(methods, NULL)));
+  PUT(info, "attributes", DICTIONARY_OBJ(copy_dictionary(attributes, NULL)));
 
   rpc_set_client_info(channel_id, info);
 }
@@ -1642,21 +1642,21 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
       api_set_error(err,
                     kErrorTypeValidation,
                     "Items in calls array must be arrays");
-      goto validation_error;
+      goto theend;
     }
     Array call = calls.items[i].data.array;
     if (call.size != 2) {
       api_set_error(err,
                     kErrorTypeValidation,
                     "Items in calls array must be arrays of size 2");
-      goto validation_error;
+      goto theend;
     }
 
     if (call.items[0].type != kObjectTypeString) {
       api_set_error(err,
                     kErrorTypeValidation,
                     "Name must be String");
-      goto validation_error;
+      goto theend;
     }
     String name = call.items[0].data.string;
 
@@ -1664,7 +1664,7 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
       api_set_error(err,
                     kErrorTypeValidation,
                     "Args must be Array");
-      goto validation_error;
+      goto theend;
     }
     Array args = call.items[1].data.array;
 
@@ -1682,11 +1682,13 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
       // error handled after loop
       break;
     }
-    if (!handler.arena_return && result.type != kObjectTypeNil) {
-      // TODO: fix to not leak memory as fuck
+    // TODO(bfredl): wastefull copy. It could be avoided to encoding to msgpack
+    // directly here. But `result` might become invalid when next api function
+    // is called in the loop.
+    ADD_C(results, copy_object(result, arena));
+    if (!handler.arena_return) {
+      api_free_object(result);
     }
-
-    ADD_C(results, result);
   }
 
   ADD_C(rv, ARRAY_OBJ(results));
@@ -1694,14 +1696,12 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
     Array errval = arena_array(arena, 3);
     ADD_C(errval, INTEGER_OBJ((Integer)i));
     ADD_C(errval, INTEGER_OBJ(nested_error.type));
-    ADD_C(errval, STRING_OBJ(cstr_to_string(nested_error.msg))); // TODO
+    ADD_C(errval, STRING_OBJ(copy_string(cstr_as_string(nested_error.msg), arena)));
     ADD_C(rv, ARRAY_OBJ(errval));
   } else {
     ADD_C(rv, NIL);
   }
-  goto theend;
 
-validation_error:
 theend:
   api_clear_error(&nested_error);
   return rv;
@@ -1754,7 +1754,7 @@ static void write_msg(String message, bool to_err)
 /// @return its argument.
 Object nvim__id(Object obj)
 {
-  return copy_object(obj);
+  return copy_object(obj, NULL);
 }
 
 /// Returns array given as argument.
@@ -1767,7 +1767,7 @@ Object nvim__id(Object obj)
 /// @return its argument.
 Array nvim__id_array(Array arr)
 {
-  return copy_object(ARRAY_OBJ(arr)).data.array;
+  return copy_array(arr, NULL);
 }
 
 /// Returns dictionary given as argument.
@@ -1780,7 +1780,7 @@ Array nvim__id_array(Array arr)
 /// @return its argument.
 Dictionary nvim__id_dictionary(Dictionary dct)
 {
-  return copy_object(DICTIONARY_OBJ(dct)).data.dictionary;
+  return copy_dictionary(dct, NULL);
 }
 
 /// Returns floating-point value given as argument.
diff --git a/src/nvim/autocmd.c b/src/nvim/autocmd.c
index 579c6c029f..5bb1bac02d 100644
--- a/src/nvim/autocmd.c
+++ b/src/nvim/autocmd.c
@@ -2051,7 +2051,7 @@ static bool call_autocmd_callback(const AutoCmd *ac, const AutoPatCmd *apc)
     PUT(data, "buf", INTEGER_OBJ(autocmd_bufnr));
 
     if (apc->data) {
-      PUT(data, "data", copy_object(*apc->data));
+      PUT(data, "data", copy_object(*apc->data, NULL));
     }
 
     int group = apc->curpat->group;
diff --git a/src/nvim/context.c b/src/nvim/context.c
index e3ae9355bf..34692cdf64 100644
--- a/src/nvim/context.c
+++ b/src/nvim/context.c
@@ -345,7 +345,7 @@ Dictionary ctx_to_dict(Context *ctx)
   PUT(rv, "jumps", ARRAY_OBJ(sbuf_to_array(ctx->jumps)));
   PUT(rv, "bufs", ARRAY_OBJ(sbuf_to_array(ctx->bufs)));
   PUT(rv, "gvars", ARRAY_OBJ(sbuf_to_array(ctx->gvars)));
-  PUT(rv, "funcs", ARRAY_OBJ(copy_array(ctx->funcs)));
+  PUT(rv, "funcs", ARRAY_OBJ(copy_array(ctx->funcs, NULL)));
 
   return rv;
 }
@@ -381,7 +381,7 @@ int ctx_from_dict(Dictionary dict, Context *ctx)
       ctx->gvars = array_to_sbuf(item.value.data.array);
     } else if (strequal(item.key.data, "funcs")) {
       types |= kCtxFuncs;
-      ctx->funcs = copy_object(item.value).data.array;
+      ctx->funcs = copy_object(item.value, NULL).data.array;
     }
   }
 
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index cacac91ee8..fe421aa272 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -359,7 +359,6 @@ static void f_api_info(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 {
   Dictionary metadata = api_metadata();
   (void)object_to_vim(DICTIONARY_OBJ(metadata), rettv, NULL);
-  api_free_dictionary(metadata);
 }
 
 /// "append(lnum, string/list)" function
diff --git a/src/nvim/eval/funcs.h b/src/nvim/eval/funcs.h
index 3aa255ed0b..adff0b2441 100644
--- a/src/nvim/eval/funcs.h
+++ b/src/nvim/eval/funcs.h
@@ -14,13 +14,13 @@ typedef void (*VimLFunc)(typval_T *args, typval_T *rvar, EvalFuncData data);
 
 /// Structure holding VimL function definition
 typedef struct {
-  char *name;        ///< Name of the function.
-  uint8_t min_argc;  ///< Minimal number of arguments.
-  uint8_t max_argc;  ///< Maximal number of arguments.
-  uint8_t base_arg;  ///< Method base arg # (1-indexed), BASE_NONE or BASE_LAST.
-  bool fast;         ///< Can be run in |api-fast| events
-  VimLFunc func;     ///< Function implementation.
-  EvalFuncData data; ///< Userdata for function implementation.
+  char *name;         ///< Name of the function.
+  uint8_t min_argc;   ///< Minimal number of arguments.
+  uint8_t max_argc;   ///< Maximal number of arguments.
+  uint8_t base_arg;   ///< Method base arg # (1-indexed), BASE_NONE or BASE_LAST.
+  bool fast;          ///< Can be run in |api-fast| events
+  VimLFunc func;      ///< Function implementation.
+  EvalFuncData data;  ///< Userdata for function implementation.
 } EvalFuncDef;
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
diff --git a/src/nvim/generators/gen_api_ui_events.lua b/src/nvim/generators/gen_api_ui_events.lua
index 93bbaab74c..f9e888c20d 100755
--- a/src/nvim/generators/gen_api_ui_events.lua
+++ b/src/nvim/generators/gen_api_ui_events.lua
@@ -125,7 +125,7 @@ for i = 1, #events do
         local param = ev.parameters[j]
         local copy = 'copy_'..param[2]
         if param[1] == 'String' then
-          send = send..'  String copy_'..param[2]..' = copy_string('..param[2]..');\n'
+          send = send..'  String copy_'..param[2]..' = copy_string('..param[2]..', NULL);\n'
           argv = argv..', '..copy..'.data, INT2PTR('..copy..'.size)'
           recv = (recv..'  String '..param[2]..
                           ' = (String){.data = argv['..argc..'],'..
@@ -134,7 +134,7 @@ for i = 1, #events do
           recv_cleanup = recv_cleanup..'  api_free_string('..param[2]..');\n'
           argc = argc+2
         elseif param[1] == 'Array' then
-          send = send..'  Array '..copy..' = copy_array('..param[2]..');\n'
+          send = send..'  Array '..copy..' = copy_array('..param[2]..', NULL);\n'
           argv = argv..', '..copy..'.items, INT2PTR('..copy..'.size)'
           recv = (recv..'  Array '..param[2]..
                           ' = (Array){.items = argv['..argc..'],'..
@@ -144,7 +144,7 @@ for i = 1, #events do
           argc = argc+2
         elseif param[1] == 'Object' then
           send = send..'  Object *'..copy..' = xmalloc(sizeof(Object));\n'
-          send = send..'  *'..copy..' = copy_object('..param[2]..');\n'
+          send = send..'  *'..copy..' = copy_object('..param[2]..', NULL);\n'
           argv = argv..', '..copy
           recv = recv..'  Object '..param[2]..' = *(Object *)argv['..argc..'];\n'
           recv_argv = recv_argv..', '..param[2]
diff --git a/src/nvim/highlight.c b/src/nvim/highlight.c
index d6a18fcf8e..c5a84c731d 100644
--- a/src/nvim/highlight.c
+++ b/src/nvim/highlight.c
@@ -903,7 +903,7 @@ Dictionary hlattrs2dict(Dictionary *hl_alloc, HlAttrs ae, bool use_rgb)
     *hl_alloc = hl;
     return hl;
   } else {
-    Dictionary allocated = copy_dictionary(hl);
+    Dictionary allocated = copy_dictionary(hl, NULL);
     kv_destroy(hl);
     return allocated;
   }
diff --git a/src/nvim/memory.c b/src/nvim/memory.c
index 7b4b104ec7..a9785fcb7c 100644
--- a/src/nvim/memory.c
+++ b/src/nvim/memory.c
@@ -575,9 +575,13 @@ void alloc_block(Arena *arena)
   blk->prev = prev_blk;
 }
 
+/// @param arena if NULL, do a global allocation. caller must then free the value!
 /// @param size if zero, will still return a non-null pointer, but not a unique one
 void *arena_alloc(Arena *arena, size_t size, bool align)
 {
+  if (!arena) {
+    return xmalloc(size);
+  }
   if (align) {
     arena->pos = (arena->pos + (ARENA_ALIGN - 1)) & ~(ARENA_ALIGN - 1);
   }
diff --git a/src/nvim/message.c b/src/nvim/message.c
index 684cf7207c..52e558a2f0 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -1086,7 +1086,7 @@ void ex_messages(void *const eap_p)
             HlMessageChunk chunk = kv_A(p->multiattr, i);
             Array content_entry = ARRAY_DICT_INIT;
             ADD(content_entry, INTEGER_OBJ(chunk.attr));
-            ADD(content_entry, STRING_OBJ(copy_string(chunk.text)));
+            ADD(content_entry, STRING_OBJ(copy_string(chunk.text, NULL)));
             ADD(content, ARRAY_OBJ(content_entry));
           }
         } else if (p->msg && p->msg[0]) {
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
index f00c62f766..d22bcb29d5 100644
--- a/src/nvim/msgpack_rpc/channel.c
+++ b/src/nvim/msgpack_rpc/channel.c
@@ -644,7 +644,7 @@ void rpc_set_client_info(uint64_t id, Dictionary info)
 
 Dictionary rpc_client_info(Channel *chan)
 {
-  return copy_dictionary(chan->rpc.info);
+  return copy_dictionary(chan->rpc.info, NULL);
 }
 
 const char *rpc_client_name(Channel *chan)
diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c
index 6afe7defe3..03cfb830e6 100644
--- a/src/nvim/tui/input.c
+++ b/src/nvim/tui/input.c
@@ -233,7 +233,7 @@ static void tinput_wait_enqueue(void **argv)
       if (ui_client_channel_id) {
         Array args = ARRAY_DICT_INIT;
         Error err = ERROR_INIT;
-        ADD(args, STRING_OBJ(copy_string(keys)));
+        ADD(args, STRING_OBJ(copy_string(keys, NULL)));
         // TODO(bfredl): could be non-blocking now with paste?
         ArenaMem res_mem = NULL;
         Object result = rpc_send_call(ui_client_channel_id, "nvim_input", args, &res_mem, &err);
diff --git a/src/nvim/types.h b/src/nvim/types.h
index 3fac97fc82..fb10bf21d9 100644
--- a/src/nvim/types.h
+++ b/src/nvim/types.h
@@ -33,7 +33,6 @@ typedef union {
   void *nullptr;
 } EvalFuncData;
 
-
 typedef handle_T NS;
 
 typedef struct expand expand_T;
diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c
index 84098e9476..809d278029 100644
--- a/src/nvim/ui_bridge.c
+++ b/src/nvim/ui_bridge.c
@@ -194,9 +194,9 @@ static void ui_bridge_suspend_event(void **argv)
 
 static void ui_bridge_option_set(UI *ui, String name, Object value)
 {
-  String copy_name = copy_string(name);
+  String copy_name = copy_string(name, NULL);
   Object *copy_value = xmalloc(sizeof(Object));
-  *copy_value = copy_object(value);
+  *copy_value = copy_object(value, NULL);
   UI_BRIDGE_CALL(ui, option_set, 4, ui, copy_name.data,
                  INT2PTR(copy_name.size), copy_value);
   // TODO(bfredl): when/if TUI/bridge teardown is refactored to use events, the