Skip to content

Commit 385b32e

Browse files
committed
Reduce risk of incomplete JSON; fix Vim crash
When editing a file with quick-lint-js' Vim plugin, quick-lint-js might produce so many errors that it flushes stdout in the middle of writing a message. If a crash soon follows, stdout isn't flushed, so the client sees invalid JSON which is difficult to successfully parse. See this strace snippet for example: write(1, "{\"qflist\": [{\"col\": 5, \"lnum\": 10, \"end_col\": 14, \"end_lnum\": 10, \"type\": \"E\", \"nr\": \"E033\", \"vcol\": 0, \"text\": \"redeclaration of global variable\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 5, \"lnum\": 11, \"end_col\": 13, \"end_lnum\": 11, \"type\": \"E\", \"nr\": \"E033\", \"vcol\": 0, \"text\": \"redeclaration of global variable\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 22, \"lnum\": 120, \"end_col\": 22, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 27, \"lnum\": 120, \"end_col\": 27, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 34, \"lnum\": 120, \"end_col\": 34, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 45, \"lnum\": 120, \"end_col\": 45, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 22, \"lnum\": 133, \"end_col\": 22, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 27, \"lnum\": 133, \"end_col\": 27, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 45, \"lnum\": 133, \"end_col\": 45, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 56, \"lnum\": 133, \"end_col\": 56, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E043\", \"vcol\": 0, \"text\": \"unexpected '\\\\' in identifier\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 59, \"lnum\": 133, \"end_col\": 59, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 25, \"lnum\": 147, \"end_col\": 25, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 26, \"lnum\": 147, \"end_col\": 26, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 40, \"lnum\": 147, \"end_col\": 40, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E054\", \"vcol\": 0, \"text\": \"unexpected token\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 53, \"lnum\": 147, \"end_col\": 53, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 58, \"lnum\": 147, \"end_col\": 58, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 64, \"lnum\": 147, \"end_col\": 118, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E040\", \"vcol\": 0, \"text\": \"unclosed string literal\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 64, \"lnum\": 147, \"end_col\": 64, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 64, \"lnum\": 147, \"end_col\": 118, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E040\", \"vcol\": 0, \"text\": \"unclosed string literal\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-documentation.mjs\"},\n{\"col\": 4, \"lnum\": 148, \"end_col\": 4, \"end_lnum\": 148, \"type\": \"E\", \"nr\": \"E025\", \"vcol\": 0, \"text\": \"missing comma between object literal entries\", \"bufnr\": 1, \"filename\": \"/tmp/vyQeHSV/1/error-doc", 4096) = 4096 write(2, "../src/./quick-lint-js/parse.h:3248: internal check failed in visit_binding_element: false\n", 91) = 91 --- SIGILL {si_signo=SIGILL, si_code=ILL_ILLOPN, si_addr=0x43004a} --- +++ killed by SIGILL (core dumped) +++ Reduce the chance of incomplete messages by flushing after writing each message. This is what text_error_reporter does implicitly (because writing '\n' to stderr flushes, at least on my machine): write(1, "{\"qflist\": [{\"col\": 5, \"lnum\": 10, \"end_col\": 14, \"end_lnum\": 10, \"type\": \"E\", \"nr\": \"E033\", \"vcol\": 0, \"text\": \"redeclaration of global variable\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 213) = 213 write(1, ",\n{\"col\": 5, \"lnum\": 11, \"end_col\": 13, \"end_lnum\": 11, \"type\": \"E\", \"nr\": \"E033\", \"vcol\": 0, \"text\": \"redeclaration of global variable\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 203) = 203 write(1, ",\n{\"col\": 22, \"lnum\": 120, \"end_col\": 22, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 27, \"lnum\": 120, \"end_col\": 27, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 34, \"lnum\": 120, \"end_col\": 34, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 45, \"lnum\": 120, \"end_col\": 45, \"end_lnum\": 120, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 22, \"lnum\": 133, \"end_col\": 22, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 27, \"lnum\": 133, \"end_col\": 27, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 45, \"lnum\": 133, \"end_col\": 45, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 56, \"lnum\": 133, \"end_col\": 56, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E043\", \"vcol\": 0, \"text\": \"unexpected '\\\\' in identifier\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 203) = 203 write(1, ",\n{\"col\": 59, \"lnum\": 133, \"end_col\": 59, \"end_lnum\": 133, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 25, \"lnum\": 147, \"end_col\": 25, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 26, \"lnum\": 147, \"end_col\": 26, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 40, \"lnum\": 147, \"end_col\": 40, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E054\", \"vcol\": 0, \"text\": \"unexpected token\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 190) = 190 write(1, ",\n{\"col\": 53, \"lnum\": 147, \"end_col\": 53, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 58, \"lnum\": 147, \"end_col\": 58, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 64, \"lnum\": 147, \"end_col\": 118, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E040\", \"vcol\": 0, \"text\": \"unclosed string literal\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 198) = 198 write(1, ",\n{\"col\": 64, \"lnum\": 147, \"end_col\": 64, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E027\", \"vcol\": 0, \"text\": \"missing semicolon after statement\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 207) = 207 write(1, ",\n{\"col\": 64, \"lnum\": 147, \"end_col\": 118, \"end_lnum\": 147, \"type\": \"E\", \"nr\": \"E040\", \"vcol\": 0, \"text\": \"unclosed string literal\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 198) = 198 write(1, ",\n{\"col\": 4, \"lnum\": 148, \"end_col\": 4, \"end_lnum\": 148, \"type\": \"E\", \"nr\": \"E025\", \"vcol\": 0, \"text\": \"missing comma between object literal entries\", \"bufnr\": 1, \"filename\": \"/tmp/vseTukR/1/error-documentation.mjs\"}", 216) = 216 write(2, "../src/./quick-lint-js/parse.h:3248: internal check failed in visit_binding_element: false\n", 91) = 91 --- SIGILL {si_signo=SIGILL, si_code=ILL_ILLOPN, si_addr=0x4300aa} --- +++ killed by SIGILL (core dumped) +++ This fixes our Vim plugin complaining about invalid JSON. I considered making QLJS_ASSERT flush stdout, but some crashes (e.g. segfaults) would still cause problems.
1 parent 927369d commit 385b32e

File tree

1 file changed

+11
-0
lines changed

1 file changed

+11
-0
lines changed

src/vim-qflist-json-error-reporter.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,17 @@ void vim_qflist_json_error_formatter::write_after_message(
135135
this->output_ << '"';
136136
}
137137
this->output_ << '}';
138+
139+
// If we don't flush, output is buffered. A lot of messages (e.g. over 4 KiB)
140+
// will fill the buffer and possibly force a flush in the middle of a message.
141+
// Then, a crash in the future (e.g. an assertion failure) will leave an
142+
// incomplete message written to the client. The client will have a hard time
143+
// extracting partial information from the incomplete JSON.
144+
//
145+
// If we flush now, it's less likely that a message ends up on the client. The
146+
// client can easily recover by adding a '}' at the end of the input to make
147+
// the incomplete JSON valid.
148+
this->output_.flush();
138149
}
139150
}
140151

0 commit comments

Comments
 (0)