Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hold stderr lock when using stdout through Console APIs. #17360

Merged
merged 1 commit into from Jun 17, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Hold stderr lock when using stdout through Console APIs.

  • Loading branch information
jdm committed Jun 16, 2017
commit b29d0c6c318ab3bede814a97a7dd83ea42d03c7b
@@ -7,6 +7,7 @@ use dom::bindings::inheritance::Castable;
use dom::bindings::str::DOMString;
use dom::globalscope::GlobalScope;
use dom::workerglobalscope::WorkerGlobalScope;
use std::io;

// https://developer.mozilla.org/en-US/docs/Web/API/Console
pub struct Console(());
@@ -27,74 +28,101 @@ impl Console {
}
}

// In order to avoid interleaving the stdout output of the Console API methods
// with stderr that could be in use on other threads, we lock stderr until
// we're finished with stdout. Since the stderr lock is reentrant, there is
// no risk of deadlock if the callback ends up trying to write to stderr for
// any reason.
fn with_stderr_lock<F>(f: F) where F: FnOnce() {
let stderr = io::stderr();
let _handle = stderr.lock();
f()
}

impl Console {
// https://developer.mozilla.org/en-US/docs/Web/API/Console/log
pub fn Log(global: &GlobalScope, messages: Vec<DOMString>) {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
}
with_stderr_lock(move || {
for message in messages {

This comment has been minimized.

@emilio

emilio Jun 16, 2017

Member

Also, I wonder whether we should split this loop (and the others) into two, one that does the printing with the lock held, and other that calls send_to_devtools.

I say that, because as far as I can tell, that function can panic and log (it uses downcast, which calls get_dom_class, which does trace!, which writes to stderr I think). I'd expect this to deadlock with RUST_LOG=trace, wouldn't it? or is the lock re-entrant? If it is, please comment on it.

This comment has been minimized.

@jdm

jdm Jun 16, 2017

Author Member

The stderr lock is reentrant, yes.

println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console
pub fn Debug(global: &GlobalScope, messages: Vec<DOMString>) {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Debug, message);
}
with_stderr_lock(move || {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Debug, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/info
pub fn Info(global: &GlobalScope, messages: Vec<DOMString>) {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Info, message);
}
with_stderr_lock(move || {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Info, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/warn
pub fn Warn(global: &GlobalScope, messages: Vec<DOMString>) {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Warn, message);
}
with_stderr_lock(move || {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Warn, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/error
pub fn Error(global: &GlobalScope, messages: Vec<DOMString>) {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Error, message);
}
with_stderr_lock(move || {
for message in messages {
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Error, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/assert
pub fn Assert(global: &GlobalScope, condition: bool, message: Option<DOMString>) {
if !condition {
let message = message.unwrap_or_else(|| DOMString::from("no message"));
println!("Assertion failed: {}", message);
Self::send_to_devtools(global, LogLevel::Error, message);
}
with_stderr_lock(move || {
if !condition {
let message = message.unwrap_or_else(|| DOMString::from("no message"));
println!("Assertion failed: {}", message);
Self::send_to_devtools(global, LogLevel::Error, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/time
pub fn Time(global: &GlobalScope, label: DOMString) {
if let Ok(()) = global.time(label.clone()) {
let message = DOMString::from(format!("{}: timer started", label));
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
}
with_stderr_lock(move || {
if let Ok(()) = global.time(label.clone()) {
let message = DOMString::from(format!("{}: timer started", label));
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
}
})
}

// https://developer.mozilla.org/en-US/docs/Web/API/Console/timeEnd
pub fn TimeEnd(global: &GlobalScope, label: DOMString) {
if let Ok(delta) = global.time_end(&label) {
let message = DOMString::from(
format!("{}: {}ms", label, delta)
);
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
};
with_stderr_lock(move || {
if let Ok(delta) = global.time_end(&label) {
let message = DOMString::from(
format!("{}: {}ms", label, delta)
);
println!("{}", message);
Self::send_to_devtools(global, LogLevel::Log, message);
};
})
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.