commit a090d43d61b5de1add5624fc55e4a9dead5b7ece
parent d6483793e1c3e337e33b53452c0e0249107d099b
Author: Fred Sundvik <fsundvik@gmail.com>
Date: Mon, 5 Feb 2024 14:39:54 +0200
fix: splitting of big UI messages
Determine the needed buffer space first, instead of trying to revert the
effect of prepare_call if message does not fit. The previous code did
not revert the full state, which caused corrupted messages to be sent.
So, rather than trying to fix all of that, with fragile and hard to read
code as a result, the code is now much more simple, although slightly
slower.
Diffstat:
2 files changed, 53 insertions(+), 49 deletions(-)
diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
@@ -585,7 +585,6 @@ static inline int write_cb(void *vdata, const char *buf, size_t len)
data->pack_totlen += len;
if (!data->temp_buf && UI_BUF_SIZE - BUF_POS(data) < len) {
- data->buf_overflow = true;
return 0;
}
@@ -595,14 +594,42 @@ static inline int write_cb(void *vdata, const char *buf, size_t len)
return 0;
}
-static bool prepare_call(UI *ui, const char *name)
+static inline int size_cb(void *vdata, const char *buf, size_t len)
+{
+ UIData *data = (UIData *)vdata;
+ if (!buf) {
+ return 0;
+ }
+
+ data->pack_totlen += len;
+ return 0;
+}
+
+static void prepare_call(UI *ui, const char *name, size_t size_needed)
{
UIData *data = ui->data;
+ size_t name_len = strlen(name);
+ const size_t overhead = name_len + 20;
+ bool oversized_message = size_needed + overhead > UI_BUF_SIZE;
- if (BUF_POS(data) > UI_BUF_SIZE - EVENT_BUF_SIZE) {
+ if (oversized_message || BUF_POS(data) > UI_BUF_SIZE - size_needed - overhead) {
remote_ui_flush_buf(ui);
}
+ if (oversized_message) {
+ // TODO(bfredl): manually testable by setting UI_BUF_SIZE to 1024 (mode_info_set)
+ data->temp_buf = xmalloc(20 + name_len + size_needed);
+ data->buf_wptr = data->temp_buf;
+ char **buf = &data->buf_wptr;
+ mpack_array(buf, 3);
+ mpack_uint(buf, 2);
+ mpack_str(buf, S_LEN("redraw"));
+ mpack_array(buf, 1);
+ mpack_array(buf, 2);
+ mpack_str(buf, name, name_len);
+ return;
+ }
+
// To optimize data transfer(especially for "grid_line"), we bundle adjacent
// calls to same method together, so only add a new call entry if the last
// method call is different from "name"
@@ -615,64 +642,42 @@ static bool prepare_call(UI *ui, const char *name)
mpack_str(buf, name, strlen(name));
data->nevents++;
data->ncalls = 1;
- return true;
+ return;
}
+}
- return false;
+static void send_oversized_message(UIData *data)
+{
+ if (data->temp_buf) {
+ size_t size = (size_t)(data->buf_wptr - data->temp_buf);
+ WBuffer *buf = wstream_new_buffer(data->temp_buf, size, 1, xfree);
+ rpc_write_raw(data->channel_id, buf);
+ data->temp_buf = NULL;
+ data->buf_wptr = data->buf;
+ data->nevents_pos = NULL;
+ }
}
/// Pushes data into UI.UIData, to be consumed later by remote_ui_flush().
static void push_call(UI *ui, const char *name, Array args)
{
UIData *data = ui->data;
- bool pending = data->nevents_pos;
- char *buf_pos_save = data->buf_wptr;
-
- bool new_event = prepare_call(ui, name);
msgpack_packer pac;
data->pack_totlen = 0;
- data->buf_overflow = false;
+ // First determine the needed size
+ msgpack_packer_init(&pac, data, size_cb);
+ msgpack_rpc_from_array(args, &pac);
+ // Then send the actual message
+ prepare_call(ui, name, data->pack_totlen);
msgpack_packer_init(&pac, data, write_cb);
msgpack_rpc_from_array(args, &pac);
- if (data->buf_overflow) {
- data->buf_wptr = buf_pos_save;
- if (new_event) {
- data->cur_event = NULL;
- data->nevents--;
- }
- if (pending) {
- remote_ui_flush_buf(ui);
- }
- size_t name_len = strlen(name);
- if (data->pack_totlen > UI_BUF_SIZE - name_len - 20) {
- // TODO(bfredl): manually testable by setting UI_BUF_SIZE to 1024 (mode_info_set)
- data->temp_buf = xmalloc(20 + name_len + data->pack_totlen);
- data->buf_wptr = data->temp_buf;
- char **buf = &data->buf_wptr;
- mpack_array(buf, 3);
- mpack_uint(buf, 2);
- mpack_str(buf, S_LEN("redraw"));
- mpack_array(buf, 1);
- mpack_array(buf, 2);
- mpack_str(buf, name, name_len);
- } else {
- prepare_call(ui, name);
- }
- data->pack_totlen = 0;
- data->buf_overflow = false;
- msgpack_rpc_from_array(args, &pac);
-
- if (data->temp_buf) {
- size_t size = (size_t)(data->buf_wptr - data->temp_buf);
- WBuffer *buf = wstream_new_buffer(data->temp_buf, size, 1, xfree);
- rpc_write_raw(data->channel_id, buf);
- data->temp_buf = NULL;
- data->buf_wptr = data->buf;
- data->nevents_pos = NULL;
- }
+ // Oversized messages need to be sent immediately
+ if (data->temp_buf) {
+ send_oversized_message(data);
}
+
data->ncalls++;
}
@@ -867,7 +872,7 @@ void remote_ui_raw_line(UI *ui, Integer grid, Integer row, Integer startcol, Int
{
UIData *data = ui->data;
if (ui->ui_ext[kUILinegrid]) {
- prepare_call(ui, "grid_line");
+ prepare_call(ui, "grid_line", EVENT_BUF_SIZE);
data->ncalls++;
char **buf = &data->buf_wptr;
@@ -896,7 +901,7 @@ void remote_ui_raw_line(UI *ui, Integer grid, Integer row, Integer startcol, Int
mpack_bool(buf, false);
remote_ui_flush_buf(ui);
- prepare_call(ui, "grid_line");
+ prepare_call(ui, "grid_line", EVENT_BUF_SIZE);
data->ncalls++;
mpack_array(buf, 5);
mpack_uint(buf, (uint32_t)grid);
diff --git a/src/nvim/ui_defs.h b/src/nvim/ui_defs.h
@@ -46,7 +46,6 @@ typedef struct {
// state for write_cb, while packing a single arglist to msgpack. This
// might fail due to buffer overflow.
size_t pack_totlen;
- bool buf_overflow;
char *temp_buf;
// We start packing the two outermost msgpack arrays before knowing the total
