Skip to content

Commit

Permalink
patch 8.1.0633: crash when out of memory while opening a terminal window
Browse files Browse the repository at this point in the history
Problem:    Crash when out of memory while opening a terminal window.
Solution:   Handle out-of-memory more gracefully.
  • Loading branch information
brammool committed Dec 24, 2018
1 parent 7a2d989 commit cd929f7
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 20 deletions.
8 changes: 8 additions & 0 deletions src/libvterm/src/state.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ static VTermState *vterm_state_new(VTerm *vt)
{
VTermState *state = vterm_allocator_malloc(vt, sizeof(VTermState));

if (state == NULL)
return NULL;
state->vt = vt;

state->rows = vt->rows;
Expand Down Expand Up @@ -1693,13 +1695,19 @@ static const VTermParserCallbacks parser_callbacks = {
on_resize /* resize */
};

/*
* Return the existing state or create a new one.
* Returns NULL when out of memory.
*/
VTermState *vterm_obtain_state(VTerm *vt)
{
VTermState *state;
if(vt->state)
return vt->state;

state = vterm_state_new(vt);
if (state == NULL)
return NULL;
vt->state = state;

state->combine_chars_size = 16;
Expand Down
28 changes: 16 additions & 12 deletions src/libvterm/src/termscreen.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "vterm_internal.h"

/* vim: set sw=2 : */
#include <stdio.h>
#include <string.h>

Expand Down Expand Up @@ -95,8 +96,7 @@ static ScreenCell *realloc_buffer(VTermScreen *screen, ScreenCell *buffer, int n
}
}

if(buffer)
vterm_allocator_free(screen->vt, buffer);
vterm_allocator_free(screen->vt, buffer);

return new_buffer;
}
Expand Down Expand Up @@ -518,8 +518,7 @@ static int resize(int new_rows, int new_cols, VTermPos *delta, void *user)
screen->rows = new_rows;
screen->cols = new_cols;

if(screen->sb_buffer)
vterm_allocator_free(screen->vt, screen->sb_buffer);
vterm_allocator_free(screen->vt, screen->sb_buffer);

screen->sb_buffer = vterm_allocator_malloc(screen->vt, sizeof(VTermScreenCell) * new_cols);

Expand Down Expand Up @@ -619,16 +618,21 @@ static VTermStateCallbacks state_cbs = {
&setlineinfo /* setlineinfo */
};

/*
* Allocate a new screen and return it.
* Return NULL when out of memory.
*/
static VTermScreen *screen_new(VTerm *vt)
{
VTermState *state = vterm_obtain_state(vt);
VTermScreen *screen;
int rows, cols;

if(!state)
if (state == NULL)
return NULL;

screen = vterm_allocator_malloc(vt, sizeof(VTermScreen));
if (screen == NULL)
return NULL;

vterm_get_size(vt, &rows, &cols);

Expand All @@ -646,10 +650,13 @@ static VTermScreen *screen_new(VTerm *vt)
screen->cbdata = NULL;

screen->buffers[0] = realloc_buffer(screen, NULL, rows, cols);

screen->buffer = screen->buffers[0];

screen->sb_buffer = vterm_allocator_malloc(screen->vt, sizeof(VTermScreenCell) * cols);
if (screen->buffer == NULL || screen->sb_buffer == NULL)
{
vterm_screen_free(screen);
return NULL;
}

vterm_state_set_callbacks(screen->state, &state_cbs, screen);

Expand All @@ -659,11 +666,8 @@ static VTermScreen *screen_new(VTerm *vt)
INTERNAL void vterm_screen_free(VTermScreen *screen)
{
vterm_allocator_free(screen->vt, screen->buffers[0]);
if(screen->buffers[1])
vterm_allocator_free(screen->vt, screen->buffers[1]);

vterm_allocator_free(screen->vt, screen->buffers[1]);
vterm_allocator_free(screen->vt, screen->sb_buffer);

vterm_allocator_free(screen->vt, screen);
}

Expand Down
20 changes: 19 additions & 1 deletion src/libvterm/src/vterm.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#define DEFINE_INLINES

/* vim: set sw=2 : */
#include "vterm_internal.h"

#include <stdio.h>
Expand Down Expand Up @@ -41,6 +42,8 @@ VTerm *vterm_new_with_allocator(int rows, int cols, VTermAllocatorFunctions *fun
/* Need to bootstrap using the allocator function directly */
VTerm *vt = (*funcs->malloc)(sizeof(VTerm), allocdata);

if (vt == NULL)
return NULL;
vt->allocator = funcs;
vt->allocdata = allocdata;

Expand All @@ -55,10 +58,21 @@ VTerm *vterm_new_with_allocator(int rows, int cols, VTermAllocatorFunctions *fun
vt->parser.strbuffer_len = 500; /* should be able to hold an OSC string */
vt->parser.strbuffer_cur = 0;
vt->parser.strbuffer = vterm_allocator_malloc(vt, vt->parser.strbuffer_len);
if (vt->parser.strbuffer == NULL)
{
vterm_allocator_free(vt, vt);
return NULL;
}

vt->outbuffer_len = 200;
vt->outbuffer_cur = 0;
vt->outbuffer = vterm_allocator_malloc(vt, vt->outbuffer_len);
if (vt->outbuffer == NULL)
{
vterm_allocator_free(vt, vt->parser.strbuffer);
vterm_allocator_free(vt, vt);
return NULL;
}

return vt;
}
Expand All @@ -82,9 +96,13 @@ INTERNAL void *vterm_allocator_malloc(VTerm *vt, size_t size)
return (*vt->allocator->malloc)(size, vt->allocdata);
}

/*
* Free "ptr" unless it is NULL.
*/
INTERNAL void vterm_allocator_free(VTerm *vt, void *ptr)
{
(*vt->allocator->free)(ptr, vt->allocdata);
if (ptr)
(*vt->allocator->free)(ptr, vt->allocdata);
}

void vterm_get_size(const VTerm *vt, int *rowsp, int *colsp)
Expand Down
32 changes: 25 additions & 7 deletions src/terminal.c
Original file line number Diff line number Diff line change
Expand Up @@ -3430,6 +3430,7 @@ set_vterm_palette(VTerm *vterm, long_u *rgb)
{
int index = 0;
VTermState *state = vterm_obtain_state(vterm);

for (; index < 16; index++)
{
VTermColor color;
Expand Down Expand Up @@ -3703,8 +3704,9 @@ static VTermAllocatorFunctions vterm_allocator = {

/*
* Create a new vterm and initialize it.
* Return FAIL when out of memory.
*/
static void
static int
create_vterm(term_T *term, int rows, int cols)
{
VTerm *vterm;
Expand All @@ -3714,15 +3716,26 @@ create_vterm(term_T *term, int rows, int cols)

vterm = vterm_new_with_allocator(rows, cols, &vterm_allocator, NULL);
term->tl_vterm = vterm;
if (vterm == NULL)
return FAIL;

// Allocate screen and state here, so we can bail out if that fails.
state = vterm_obtain_state(vterm);
screen = vterm_obtain_screen(vterm);
if (state == NULL || screen == NULL)
{
vterm_free(vterm);
return FAIL;
}

vterm_screen_set_callbacks(screen, &screen_callbacks, term);
/* TODO: depends on 'encoding'. */
vterm_set_utf8(vterm, 1);

init_default_colors(term);

vterm_state_set_default_colors(
vterm_obtain_state(vterm),
state,
&term->tl_default_color.fg,
&term->tl_default_color.bg);

Expand All @@ -3746,9 +3759,10 @@ create_vterm(term_T *term, int rows, int cols)
#else
value.boolean = 0;
#endif
state = vterm_obtain_state(vterm);
vterm_state_set_termprop(state, VTERM_PROP_CURSORBLINK, &value);
vterm_state_set_unrecognised_fallbacks(state, &parser_fallbacks, term);

return OK;
}

/*
Expand Down Expand Up @@ -5629,7 +5643,8 @@ term_and_job_init(
vim_free(cwd_wchar);
vim_free(env_wchar);

create_vterm(term, term->tl_rows, term->tl_cols);
if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
goto failed;

#if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
if (opt->jo_set2 & JO2_ANSI_COLORS)
Expand Down Expand Up @@ -5710,7 +5725,8 @@ create_pty_only(term_T *term, jobopt_T *options)
char in_name[80], out_name[80];
channel_T *channel = NULL;

create_vterm(term, term->tl_rows, term->tl_cols);
if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
return FAIL;

vim_snprintf(in_name, sizeof(in_name), "\\\\.\\pipe\\vim-%d-in-%d",
GetCurrentProcessId(),
Expand Down Expand Up @@ -5822,7 +5838,8 @@ term_and_job_init(
jobopt_T *opt,
jobopt_T *orig_opt UNUSED)
{
create_vterm(term, term->tl_rows, term->tl_cols);
if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
return FAIL;

#if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
if (opt->jo_set2 & JO2_ANSI_COLORS)
Expand All @@ -5844,7 +5861,8 @@ term_and_job_init(
static int
create_pty_only(term_T *term, jobopt_T *opt)
{
create_vterm(term, term->tl_rows, term->tl_cols);
if (create_vterm(term, term->tl_rows, term->tl_cols) == FAIL)
return FAIL;

term->tl_job = job_alloc();
if (term->tl_job == NULL)
Expand Down
2 changes: 2 additions & 0 deletions src/version.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
633,
/**/
632,
/**/
Expand Down

3 comments on commit cd929f7

@leonerd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a little concerned about the divergence between vim's private clone of libvterm and the upstream original. Could you attempt to extract this change out for the original upstream too? I suspect at some point I'd like to try to recombine them again to avoid this ongoing divergence.

@brammool
Copy link
Contributor Author

@brammool brammool commented on cd929f7 Dec 24, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisbra
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you attempt to extract this change out for the original upstream too?

Isn't this simply git log -v -p -1 cd929f7ba8cc5b6d6dcf35c8b34124e969fed6b8 -- src/libvterm

For comparing the directories, you would probably do something alike git diff --no-index with the two toplevel libvterm directories.

Please sign in to comment.