mirror of
https://github.com/tree-sitter/tree-sitter.git
synced 2026-09-10 07:36:22 -04:00
fix(rust)!: require parser loggers to be Send + 'static
`Parser::set_logger` boxes the logger callback into a type-erased C pointer, but `Parser` carries no lifetime parameter. This allows for two kinds of UB: - Use-after-free: a logger could capture a non-`'static` borrow, let the borrowed value drop, and then dangle the next time the parser logged. - Data race: a logger could capture a `!Send` value such as an `Rc`. The parser could then be moved to another thread and logged from there while the original thread still held a clone, racing on the reference count. As a fix, we just require that logger is `Send + 'static`. The alternative here is to attach a lifetime parameter to `Parser`, but this is highly breaking for what is mostly a debugging utility. As an escape hatch `set_logger_unchecked` is added to the public API as an `unsafe fn` to correctly communicate the risks and invariants that must be held.
This commit is contained in:
parent
65b5b03c94
commit
d205dc92de
|
|
@ -66,9 +66,13 @@ fn test_parsing_with_logging() {
|
|||
parser.set_language(&get_language("rust")).unwrap();
|
||||
|
||||
let mut messages = Vec::new();
|
||||
parser.set_logger(Some(Box::new(|log_type, message| {
|
||||
messages.push((log_type, message.to_string()));
|
||||
})));
|
||||
// SAFETY: the logger borrows `messages` and is only invoked during the
|
||||
// `parse` call below while `messages` is in scope.
|
||||
unsafe {
|
||||
parser.set_logger_unchecked(Some(Box::new(|log_type, message| {
|
||||
messages.push((log_type, message.to_string()));
|
||||
})));
|
||||
}
|
||||
|
||||
parser
|
||||
.parse(
|
||||
|
|
@ -1785,11 +1789,15 @@ fn test_parsing_with_scanner_logging() {
|
|||
.unwrap();
|
||||
|
||||
let mut found = false;
|
||||
parser.set_logger(Some(Box::new(|log_type, message| {
|
||||
if log_type == LogType::Lex && message == "Found a percent string" {
|
||||
found = true;
|
||||
}
|
||||
})));
|
||||
// SAFETY: the logger borrows `found` and is only invoked during the `parse`
|
||||
// call below, while `found` is in scope.
|
||||
unsafe {
|
||||
parser.set_logger_unchecked(Some(Box::new(|log_type, message| {
|
||||
if log_type == LogType::Lex && message == "Found a percent string" {
|
||||
found = true;
|
||||
}
|
||||
})));
|
||||
}
|
||||
|
||||
let source_code = "x + %(sup (external) scanner?)";
|
||||
|
||||
|
|
|
|||
|
|
@ -301,7 +301,10 @@ pub enum LogType {
|
|||
type FieldId = NonZeroU16;
|
||||
|
||||
/// A callback that receives log messages during parsing.
|
||||
type Logger<'a> = Box<dyn FnMut(LogType, &str) + 'a>;
|
||||
type Logger = Box<dyn FnMut(LogType, &str) + Send + 'static>;
|
||||
|
||||
/// A callback that receives log messages during parsing, with relaxed constraints.
|
||||
type UnsafeLogger<'a> = Box<dyn FnMut(LogType, &str) + Send + 'a>;
|
||||
|
||||
/// A callback that receives the parse state during parsing.
|
||||
type ParseProgressCallback<'a> = &'a mut dyn FnMut(&ParseState) -> ControlFlow<()>;
|
||||
|
|
@ -767,8 +770,24 @@ impl Parser {
|
|||
}
|
||||
|
||||
/// Set the logging callback that the parser should use during parsing.
|
||||
///
|
||||
/// To log through a callback that borrows non-`'static` data, see
|
||||
/// [`set_logger_unchecked`](Parser::set_logger_unchecked).
|
||||
#[doc(alias = "ts_parser_set_logger")]
|
||||
pub fn set_logger(&mut self, logger: Option<Logger>) {
|
||||
// SAFETY: `Logger` is `Send + 'static`
|
||||
unsafe { self.set_logger_unchecked(logger) };
|
||||
}
|
||||
|
||||
/// Set the logging callback, allowing the callback to borrow non-`'static`
|
||||
/// data. See [`set_logger`](Parser::set_logger).
|
||||
///
|
||||
/// # Safety
|
||||
///
|
||||
/// Any data borrowed by `logger` must remain valid until the logger is
|
||||
/// removed. Equivalently, the parser must not be used to parse once the
|
||||
/// borrowed data has gone out of scope.
|
||||
pub unsafe fn set_logger_unchecked(&mut self, logger: Option<UnsafeLogger<'_>>) {
|
||||
let prev_logger = unsafe { ffi::ts_parser_logger(self.0.as_ptr()) };
|
||||
if !prev_logger.payload.is_null() {
|
||||
drop(unsafe { Box::from_raw(prev_logger.payload.cast::<Logger>()) });
|
||||
|
|
|
|||
Loading…
Reference in a new issue