Skip to content

Commit e897cca

Browse files
committed
terminal: Don't store a buf_T reference in the terminal struct
Since vimscript can close buffers at any time, it is possible that a refresh_timer_cb will be called with an invalid buffer, but there's no way to detect this if only a reference is stored because the memory can be reused by the allocator. Use buf_T->handle which is guaranteed to be unique.
1 parent 47cbbc0 commit e897cca

File tree

1 file changed

+61
-48
lines changed

1 file changed

+61
-48
lines changed

src/nvim/terminal.c

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
#include "nvim/event/time.h"
7272
#include "nvim/os/input.h"
7373
#include "nvim/api/private/helpers.h"
74+
#include "nvim/api/private/handle.h"
7475

7576
#ifdef INCLUDE_GENERATED_DECLARATIONS
7677
# include "terminal.c.generated.h"
@@ -113,7 +114,10 @@ struct terminal {
113114
// window height has increased) and must be deleted from the terminal buffer
114115
int sb_pending;
115116
// buf_T instance that acts as a "drawing surface" for libvterm
116-
buf_T *buf;
117+
// we can't store a direct reference to the buffer because the
118+
// refresh_timer_cb may be called after the buffer was freed, and there's
119+
// no way to know if the memory was reused.
120+
uint64_t buf_handle;
117121
// program exited
118122
bool closed, destroy;
119123
// some vterm properties
@@ -197,7 +201,7 @@ Terminal *terminal_open(TerminalOptions opts)
197201
rv->opts = opts;
198202
rv->cursor.visible = true;
199203
// Associate the terminal instance with the new buffer
200-
rv->buf = curbuf;
204+
rv->buf_handle = curbuf->handle;
201205
curbuf->terminal = rv;
202206
// Create VTerm
203207
rv->vt = vterm_new(opts.height, opts.width);
@@ -215,7 +219,7 @@ Terminal *terminal_open(TerminalOptions opts)
215219
// have as many lines as screen rows when refresh_scrollback is called
216220
rv->invalid_start = 0;
217221
rv->invalid_end = opts.height;
218-
refresh_screen(rv);
222+
refresh_screen(rv, curbuf);
219223
set_option_value((uint8_t *)"buftype", 0, (uint8_t *)"terminal", OPT_LOCAL);
220224
// some sane settings for terminal buffers
221225
set_option_value((uint8_t *)"wrap", false, NULL, OPT_LOCAL);
@@ -232,7 +236,7 @@ Terminal *terminal_open(TerminalOptions opts)
232236
// - SCROLLBACK_BUFFER_DEFAULT_SIZE
233237
//
234238
// but limit to 100k.
235-
int size = get_config_int(rv, "terminal_scrollback_buffer_size");
239+
int size = get_config_int("terminal_scrollback_buffer_size");
236240
rv->sb_size = size > 0 ? (size_t)size : SCROLLBACK_BUFFER_DEFAULT_SIZE;
237241
rv->sb_size = MIN(rv->sb_size, 100000);
238242
rv->sb_buffer = xmalloc(sizeof(ScrollbackLine *) * rv->sb_size);
@@ -246,7 +250,7 @@ Terminal *terminal_open(TerminalOptions opts)
246250
RgbValue color_val = -1;
247251
char var[64];
248252
snprintf(var, sizeof(var), "terminal_color_%d", i);
249-
char *name = get_config_string(rv, var);
253+
char *name = get_config_string(var);
250254
if (name) {
251255
color_val = name_to_color((uint8_t *)name);
252256
xfree(name);
@@ -276,11 +280,14 @@ void terminal_close(Terminal *term, char *msg)
276280
term->forward_mouse = false;
277281
term->closed = true;
278282
if (!msg || exiting) {
283+
buf_T *buf = handle_get_buffer(term->buf_handle);
279284
// If no msg was given, this was called by close_buffer(buffer.c). Or if
280285
// exiting, we must inform the buffer the terminal no longer exists so that
281286
// close_buffer() doesn't call this again.
282-
term->buf->terminal = NULL;
283-
term->buf = NULL;
287+
term->buf_handle = 0;
288+
if (buf) {
289+
buf->terminal = NULL;
290+
}
284291
if (!term->refcount) {
285292
// We should not wait for the user to press a key.
286293
term->opts.close_cb(term->opts.data);
@@ -311,7 +318,7 @@ void terminal_resize(Terminal *term, uint16_t width, uint16_t height)
311318
// The new width/height are the minimum for all windows that display the
312319
// terminal in the current tab.
313320
FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
314-
if (!wp->w_closing && wp->w_buffer == term->buf) {
321+
if (!wp->w_closing && wp->w_buffer->terminal == term) {
315322
width = (uint16_t)MIN(width, (uint16_t)(wp->w_width - win_col_off(wp)));
316323
height = (uint16_t)MIN(height, (uint16_t)wp->w_height);
317324
}
@@ -333,7 +340,8 @@ void terminal_resize(Terminal *term, uint16_t width, uint16_t height)
333340

334341
void terminal_enter(void)
335342
{
336-
Terminal *term = curbuf->terminal;
343+
buf_T *buf = curbuf;
344+
Terminal *term = buf->terminal;
337345
assert(term && "should only be called when curbuf has a terminal");
338346

339347
// Ensure the terminal is properly sized.
@@ -348,7 +356,7 @@ void terminal_enter(void)
348356
bool save_mapped_ctrl_c = mapped_ctrl_c;
349357
mapped_ctrl_c = true;
350358
// go to the bottom when the terminal is focused
351-
adjust_topline(term, false);
359+
adjust_topline(term, buf, false);
352360
// erase the unfocused cursor
353361
invalidate_terminal(term, term->cursor.row, term->cursor.row + 1);
354362
showmode();
@@ -359,7 +367,7 @@ void terminal_enter(void)
359367

360368
bool got_bs = false; // True if the last input was <C-\>
361369

362-
while (term->buf == curbuf) {
370+
while (curbuf->handle == term->buf_handle) {
363371
input_enable_events();
364372
c = safe_vgetc();
365373
input_disable_events();
@@ -386,7 +394,7 @@ void terminal_enter(void)
386394
term->refcount++;
387395
queue_process_events(loop.events);
388396
term->refcount--;
389-
if (term->buf == NULL) {
397+
if (term->buf_handle == 0) {
390398
close = true;
391399
goto end;
392400
}
@@ -421,10 +429,10 @@ void terminal_enter(void)
421429
invalidate_terminal(term, term->cursor.row, term->cursor.row + 1);
422430
mapped_ctrl_c = save_mapped_ctrl_c;
423431
unshowmode(true);
424-
redraw(term->buf != curbuf);
432+
redraw(buf != curbuf);
425433
ui_busy_stop();
426434
if (close) {
427-
bool wipe = term->buf != NULL;
435+
bool wipe = term->buf_handle != 0;
428436
term->opts.close_cb(term->opts.data);
429437
if (wipe) {
430438
do_cmdline_cmd("bwipeout!");
@@ -434,10 +442,11 @@ void terminal_enter(void)
434442

435443
void terminal_destroy(Terminal *term)
436444
{
437-
if (term->buf) {
438-
term->buf->terminal = NULL;
445+
buf_T *buf = handle_get_buffer(term->buf_handle);
446+
if (buf) {
447+
term->buf_handle = 0;
448+
buf->terminal = NULL;
439449
}
440-
term->buf = NULL;
441450

442451
if (!term->refcount) {
443452
if (pmap_has(ptr_t)(invalidated_terminals, term)) {
@@ -592,8 +601,9 @@ static int term_settermprop(VTermProp prop, VTermValue *val, void *data)
592601
break;
593602

594603
case VTERM_PROP_TITLE: {
604+
buf_T *buf = handle_get_buffer(term->buf_handle);
595605
Error err;
596-
api_free_object(dict_set_value(term->buf->b_vars,
606+
api_free_object(dict_set_value(buf->b_vars,
597607
cstr_as_string("term_title"),
598608
STRING_OBJ(cstr_as_string(val->string)),
599609
&err));
@@ -781,7 +791,7 @@ static bool send_mouse_event(Terminal *term, int c)
781791
int row = mouse_row, col = mouse_col;
782792
win_T *mouse_win = mouse_find_win(&row, &col);
783793

784-
if (term->forward_mouse && mouse_win->w_buffer == term->buf) {
794+
if (term->forward_mouse && mouse_win->w_buffer->terminal == term) {
785795
// event in the terminal window and mouse events was enabled by the
786796
// program. translate and forward the event
787797
int button;
@@ -906,22 +916,23 @@ static void invalidate_terminal(Terminal *term, int start_row, int end_row)
906916
static void refresh_terminal(Terminal *term)
907917
{
908918
// TODO(SplinterOfChaos): Find the condition that makes term->buf invalid.
919+
buf_T *buf = handle_get_buffer(term->buf_handle);
909920
bool valid = true;
910-
if (!term->buf || !(valid = buf_valid(term->buf))) {
921+
if (!buf || !(valid = buf_valid(buf))) {
911922
// destroyed by `close_buffer`. Dont do anything else
912923
if (!valid) {
913-
term->buf = NULL;
924+
term->buf_handle = 0;
914925
}
915926
return;
916927
}
917928
bool pending_resize = term->pending_resize;
918-
WITH_BUFFER(term->buf, {
919-
refresh_size(term);
920-
refresh_scrollback(term);
921-
refresh_screen(term);
922-
redraw_buf_later(term->buf, NOT_VALID);
929+
WITH_BUFFER(buf, {
930+
refresh_size(term, buf);
931+
refresh_scrollback(term, buf);
932+
refresh_screen(term, buf);
933+
redraw_buf_later(buf, NOT_VALID);
923934
});
924-
adjust_topline(term, pending_resize);
935+
adjust_topline(term, buf, pending_resize);
925936
}
926937
// libuv timer callback. This will enqueue on_refresh to be processed as an
927938
// event.
@@ -946,7 +957,7 @@ static void refresh_timer_cb(TimeWatcher *watcher, void *data)
946957
refresh_pending = false;
947958
}
948959

949-
static void refresh_size(Terminal *term)
960+
static void refresh_size(Terminal *term, buf_T *buf)
950961
{
951962
if (!term->pending_resize || term->closed) {
952963
return;
@@ -961,7 +972,7 @@ static void refresh_size(Terminal *term)
961972
}
962973

963974
// Refresh the scrollback of a invalidated terminal
964-
static void refresh_scrollback(Terminal *term)
975+
static void refresh_scrollback(Terminal *term, buf_T *buf)
965976
{
966977
int width, height;
967978
vterm_get_size(term->vt, &height, &width);
@@ -971,29 +982,29 @@ static void refresh_scrollback(Terminal *term)
971982
// became full and libvterm had to push all rows up. Convert the first
972983
// pending scrollback row into a string and append it just above the visible
973984
// section of the buffer
974-
if (((int)term->buf->b_ml.ml_line_count - height) >= (int)term->sb_size) {
985+
if (((int)buf->b_ml.ml_line_count - height) >= (int)term->sb_size) {
975986
// scrollback full, delete lines at the top
976987
ml_delete(1, false);
977988
deleted_lines(1, 1);
978989
}
979990
fetch_row(term, -term->sb_pending, width);
980-
int buf_index = (int)term->buf->b_ml.ml_line_count - height;
991+
int buf_index = (int)buf->b_ml.ml_line_count - height;
981992
ml_append(buf_index, (uint8_t *)term->textbuf, 0, false);
982993
appended_lines(buf_index, 1);
983994
term->sb_pending--;
984995
}
985996

986997
// Remove extra lines at the bottom
987998
int max_line_count = (int)term->sb_current + height;
988-
while (term->buf->b_ml.ml_line_count > max_line_count) {
989-
ml_delete(term->buf->b_ml.ml_line_count, false);
990-
deleted_lines(term->buf->b_ml.ml_line_count, 1);
999+
while (buf->b_ml.ml_line_count > max_line_count) {
1000+
ml_delete(buf->b_ml.ml_line_count, false);
1001+
deleted_lines(buf->b_ml.ml_line_count, 1);
9911002
}
9921003
}
9931004

9941005
// Refresh the screen(visible part of the buffer when the terminal is
9951006
// focused) of a invalidated terminal
996-
static void refresh_screen(Terminal *term)
1007+
static void refresh_screen(Terminal *term, buf_T *buf)
9971008
{
9981009
int changed = 0;
9991010
int added = 0;
@@ -1008,7 +1019,7 @@ static void refresh_screen(Terminal *term)
10081019
r < term->invalid_end; r++, linenr++) {
10091020
fetch_row(term, r, width);
10101021

1011-
if (linenr <= term->buf->b_ml.ml_line_count) {
1022+
if (linenr <= buf->b_ml.ml_line_count) {
10121023
ml_replace(linenr, (uint8_t *)term->textbuf, true);
10131024
changed++;
10141025
} else {
@@ -1072,20 +1083,20 @@ static void redraw(bool restore_cursor)
10721083
ui_flush();
10731084
}
10741085

1075-
static void adjust_topline(Terminal *term, bool force)
1086+
static void adjust_topline(Terminal *term, buf_T *buf, bool force)
10761087
{
10771088
int height, width;
10781089
vterm_get_size(term->vt, &height, &width);
10791090
FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
1080-
if (wp->w_buffer == term->buf) {
1091+
if (wp->w_buffer == buf) {
10811092
// for every window that displays a terminal, ensure the cursor is in a
10821093
// valid line
1083-
wp->w_cursor.lnum = MIN(wp->w_cursor.lnum, term->buf->b_ml.ml_line_count);
1084-
if (force || curbuf != term->buf || is_focused(term)) {
1094+
wp->w_cursor.lnum = MIN(wp->w_cursor.lnum, buf->b_ml.ml_line_count);
1095+
if (force || curbuf != buf || is_focused(term)) {
10851096
// if the terminal is not in the current window or if it's focused,
10861097
// adjust topline/cursor so the window will "follow" the terminal
10871098
// output
1088-
wp->w_cursor.lnum = term->buf->b_ml.ml_line_count;
1099+
wp->w_cursor.lnum = buf->b_ml.ml_line_count;
10891100
set_topline(wp, MAX(wp->w_cursor.lnum - height + 1, 1));
10901101
}
10911102
}
@@ -1104,33 +1115,35 @@ static int linenr_to_row(Terminal *term, int linenr)
11041115

11051116
static bool is_focused(Terminal *term)
11061117
{
1107-
return State & TERM_FOCUS && curbuf == term->buf;
1118+
return State & TERM_FOCUS && curbuf->terminal == term;
11081119
}
11091120

1110-
#define GET_CONFIG_VALUE(t, k, o) \
1121+
#define GET_CONFIG_VALUE(k, o) \
11111122
do { \
11121123
Error err; \
1113-
o = dict_get_value(t->buf->b_vars, cstr_as_string(k), &err); \
1124+
/* Only called from terminal_open where curbuf->terminal is the */ \
1125+
/* context */ \
1126+
o = dict_get_value(curbuf->b_vars, cstr_as_string(k), &err); \
11141127
if (o.type == kObjectTypeNil) { \
11151128
o = dict_get_value(&globvardict, cstr_as_string(k), &err); \
11161129
} \
11171130
} while (0)
11181131

1119-
static char *get_config_string(Terminal *term, char *key)
1132+
static char *get_config_string(char *key)
11201133
{
11211134
Object obj;
1122-
GET_CONFIG_VALUE(term, key, obj);
1135+
GET_CONFIG_VALUE(key, obj);
11231136
if (obj.type == kObjectTypeString) {
11241137
return obj.data.string.data;
11251138
}
11261139
api_free_object(obj);
11271140
return NULL;
11281141
}
11291142

1130-
static int get_config_int(Terminal *term, char *key)
1143+
static int get_config_int(char *key)
11311144
{
11321145
Object obj;
1133-
GET_CONFIG_VALUE(term, key, obj);
1146+
GET_CONFIG_VALUE(key, obj);
11341147
if (obj.type == kObjectTypeInteger) {
11351148
return (int)obj.data.integer;
11361149
}

0 commit comments

Comments
 (0)