Auto merge of #26196 - jdm:stdout-log, r=Manishearth

Dump all UWP logging output to a file

These changes ensure that we keep getting debug output in the VS output window, but we also write each message to a file in the app's local data directory. These changes also extend the C API a bit to support more generic logging facilities, removing some of the Windows-specific nature of the VSLogger. Fixes part of #23813.

ports/libsimpleservo/capi/src/lib.rs

@@ -27,17 +27,20 @@ use std::os::raw::{c_char, c_uint, c_void};
 use std::panic::{self, UnwindSafe};
 use std::slice;
 use std::str::FromStr;
-use std::sync::RwLock;
+use std::sync::{Mutex, RwLock};
 
 extern "C" fn default_panic_handler(msg: *const c_char) {
     let c_str: &CStr = unsafe { CStr::from_ptr(msg) };
     error!("{}", c_str.to_str().unwrap());
 }
 
+type LogHandlerFn = extern "C" fn(buffer: *const c_char, len: u32);
+
 lazy_static! {
     static ref ON_PANIC: RwLock<extern "C" fn(*const c_char)> = RwLock::new(default_panic_handler);
     static ref SERVO_VERSION: CString =
         CString::new(simpleservo::servo_version()).expect("Can't create string");
+    pub(crate) static ref OUTPUT_LOG_HANDLER: Mutex<Option<LogHandlerFn>> = Mutex::new(None);
 }
 
 #[no_mangle]
@@ -64,13 +67,13 @@ fn catch_any_panic<T, F: FnOnce() -> T + UnwindSafe>(function: F) -> T {
 }
 
 #[cfg(not(target_os = "windows"))]
-fn redirect_stdout_stderr() -> Result<(), String> {
+fn redirect_stdout_stderr(_handler: LogHandlerFn) -> Result<(), String> {
     Ok(())
 }
 
 #[cfg(target_os = "windows")]
-fn redirect_stdout_stderr() -> Result<(), String> {
+fn redirect_stdout_stderr(handler: LogHandlerFn) -> Result<(), String> {
-    do_redirect_stdout_stderr().map_err(|()| {
+    do_redirect_stdout_stderr(handler).map_err(|()| {
         format!("GetLastError() = {}", unsafe {
             winapi::um::errhandlingapi::GetLastError()
         })
@@ -83,10 +86,9 @@ fn redirect_stdout_stderr() -> Result<(), String> {
 // Return Value: Result<(), String>
 //              Ok() - stdout and stderr redirects.
 //              Err(str) - The Err value can contain the string value of GetLastError.
-fn do_redirect_stdout_stderr() -> Result<(), ()> {
+fn do_redirect_stdout_stderr(handler: LogHandlerFn) -> Result<(), ()> {
     use std::thread;
     use winapi::shared;
-    use winapi::um::debugapi;
     use winapi::um::handleapi;
     use winapi::um::minwinbase;
     use winapi::um::namedpipeapi;
@@ -163,24 +165,20 @@ fn do_redirect_stdout_stderr() -> Result<(), ()> {
         }
 
         // Spawn a thread.  The thread will redirect all STDOUT and STDERR messages
-        // to OutputDebugString()
+        // to the provided handler function.
-        let _handler = thread::spawn(move || {
+        let _handler = thread::spawn(move || loop {
-            loop {
+            let mut read_buf: [i8; BUF_LENGTH] = [0; BUF_LENGTH];
-                let mut read_buf: [i8; BUF_LENGTH] = [0; BUF_LENGTH];
 
-                let result = libc::read(
+            let result = libc::read(
-                    h_read_pipe_fd,
+                h_read_pipe_fd,
-                    read_buf.as_mut_ptr() as *mut _,
+                read_buf.as_mut_ptr() as *mut _,
-                    read_buf.len() as u32 - 1,
+                read_buf.len() as u32 - 1,
-                );
+            );
 
-                if result == -1 {
+            if result == -1 {
-                    break;
+                break;
-                }
-
-                // Write to Debug port.
-                debugapi::OutputDebugStringA(read_buf.as_mut_ptr() as winnt::LPSTR);
             }
+            handler(read_buf.as_ptr(), result as u32);
         });
     }
 
@@ -232,6 +230,7 @@ pub struct CHostCallbacks {
     pub on_devtools_started: extern "C" fn(result: CDevtoolsServerState, port: c_uint),
     pub show_context_menu:
         extern "C" fn(title: *const c_char, items_list: *const *const c_char, items_size: u32),
+    pub on_log_output: extern "C" fn(buffer: *const c_char, buffer_length: u32),
 }
 
 /// Servo options
@@ -422,9 +421,10 @@ unsafe fn init(
         slice::from_raw_parts(opts.vslogger_mod_list, opts.vslogger_mod_size as usize)
     };
 
+    *OUTPUT_LOG_HANDLER.lock().unwrap() = Some(callbacks.on_log_output);
     init_logger(logger_modules, logger_level);
 
-    if let Err(reason) = redirect_stdout_stderr() {
+    if let Err(reason) = redirect_stdout_stderr(callbacks.on_log_output) {
         warn!("Error redirecting stdout/stderr: {}", reason);
     }
 

ports/libsimpleservo/capi/src/vslogger.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use crate::OUTPUT_LOG_HANDLER;
 use log::{self, Metadata, Record};
 use std::sync::{Arc, Mutex};
 
@@ -9,10 +10,6 @@ lazy_static! {
     pub static ref LOG_MODULE_FILTERS: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(vec![]));
 }
 
-extern "C" {
-    fn OutputDebugStringA(s: *const u8);
-}
-
 pub struct VSLogger;
 
 impl log::Log for VSLogger {
@@ -33,9 +30,9 @@ impl log::Log for VSLogger {
                 record.target(),
                 record.args()
             );
-            unsafe {
+            if let Some(handler) = OUTPUT_LOG_HANDLER.lock().unwrap().as_ref() {
-                OutputDebugStringA(log.as_ptr());
+                (handler)(log.as_ptr() as _, log.len() as u32);
-            };
+            }
         }
     }
 

support/hololens/ServoApp/ServoControl/Servo.cpp

@@ -36,6 +36,10 @@ void on_animating_changed(bool aAnimating) {
 }
 
 void on_panic(const char *backtrace) {
+  if (sLogHandle != INVALID_HANDLE_VALUE) {
+    CloseHandle(sLogHandle);
+    sLogHandle = INVALID_HANDLE_VALUE;
+  }
   throw hresult_error(E_FAIL, char2hstring(backtrace));
 }
 
@@ -86,6 +90,23 @@ void on_devtools_started(Servo::DevtoolsServerState result,
       result == Servo::DevtoolsServerState::Started, port);
 }
 
+void on_log_output(const char *buffer, uint32_t buffer_length) {
+  OutputDebugStringA(buffer);
+
+  if (sLogHandle == INVALID_HANDLE_VALUE) {
+    return;
+  }
+
+  DWORD bytesWritten;
+  auto writeResult =
+      WriteFile(sLogHandle, buffer, buffer_length, &bytesWritten, nullptr);
+
+  if (writeResult == FALSE || bytesWritten != buffer_length)
+    throw std::runtime_error(
+        "Failed to write log message to the log file: error code " +
+        std::to_string(GetLastError()));
+}
+
 Servo::PromptResult prompt_ok_cancel(const char *message, bool trusted) {
   return sServo->Delegate().OnServoPromptOkCancel(char2hstring(message),
                                                   trusted);
@@ -141,6 +162,22 @@ Servo::Servo(hstring url, hstring args, GLsizei width, GLsizei height,
 
   sServo = this; // FIXME;
 
+#ifdef _DEBUG
+  auto current = winrt::Windows::Storage::ApplicationData::Current();
+  auto filePath = std::wstring(current.LocalFolder().Path()) + L"\\stdout.txt";
+  sLogHandle =
+      CreateFile2(filePath.c_str(), GENERIC_WRITE, 0, CREATE_ALWAYS, nullptr);
+  if (sLogHandle == INVALID_HANDLE_VALUE)
+    throw std::runtime_error("Failed to open the log file: error code " +
+                             std::to_string(GetLastError()));
+
+  if (SetFilePointer(sLogHandle, 0, nullptr, FILE_END) ==
+      INVALID_SET_FILE_POINTER)
+    throw std::runtime_error(
+        "Failed to set file pointer to the end of file: error code " +
+        std::to_string(GetLastError()));
+#endif
+
   capi::CHostCallbacks c;
   c.flush = &flush;
   c.make_current = &make_current;
@@ -164,13 +201,18 @@ Servo::Servo(hstring url, hstring args, GLsizei width, GLsizei height,
   c.prompt_input = &prompt_input;
   c.on_devtools_started = &on_devtools_started;
   c.show_context_menu = &show_context_menu;
+  c.on_log_output = &on_log_output;
 
   capi::register_panic_handler(&on_panic);
 
   capi::init_with_egl(o, &wakeup, c);
 }
 
-Servo::~Servo() { sServo = nullptr; }
+Servo::~Servo() {
+  sServo = nullptr;
+  if (sLogHandle != INVALID_HANDLE_VALUE)
+    CloseHandle(sLogHandle);
+}
 
 winrt::hstring char2hstring(const char *c_str) {
   // FIXME: any better way of doing this?

support/hololens/ServoApp/ServoControl/Servo.h

@@ -119,5 +119,6 @@ protected:
 // pointer as callback in Servo, and these functions need a way to get
 // the Servo instance. See https://github.com/servo/servo/issues/22967
 static Servo *sServo = nullptr;
+static HANDLE sLogHandle = INVALID_HANDLE_VALUE;
 
 } // namespace winrt::servo