Skip to content

Commit 1d00c36

Browse files
edwintoroklindig
authored andcommitted
Fix backtraces in xenopsd-xc
A failing HOST.stat call logged just the exception, but not the backtrace: ``` Sep 1 10:55:07 localhost xenopsd-xc: [error||72 ||backtrace] dbsync (update_env) R:a771d46f95c9 failed with exception Xenctrlext.Unix_error(57, "105: No buffer space availa ble") Sep 1 10:55:07 localhost xenopsd-xc: [error||72 ||backtrace] Raised Xenctrlext.Unix_error(57, "105: No buffer space available") Sep 1 10:55:07 localhost xenopsd-xc: [error||72 ||backtrace] 1/1 xenopsd-xc Raised at file (Thread 72 has no backtrace table. Was with_backtraces called?, line 0 Sep 1 10:55:07 localhost xenopsd-xc: [error||72 ||backtrace] ``` Although with_backtraces was definitely called by Debug.with_thread_associated: ``` let stat _ dbg = Debug.with_thread_associated dbg (fun () -> debug "HOST.stat" ; let module B = (val get_backend () : S) in B.HOST.stat ()) () ``` This is because `with_backtraces` removes the thread-local backtrace table that it has created on exit and returns an exception together with its backtrace. The backtrace was getting ignored, and re-queried in Debug.log_backtrace_exn. However that only works if Debug.with_* calls are nested. Fix this so we always use the backtrace if available instead of querying it. log_backtrace can also be called externally, so make sure we still attempt to retrieve the backtrace from the exception if the backtrace is empty. After the fix we now get a proper backtrace: ``` Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] dbsync (update_env) R:531918011f4b failed with exception Xenctrlext.Unix_error(57, "105: No buffer space availa ble") Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] Raised Xenctrlext.Unix_error(57, "105: No buffer space available") Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] 1/3 xenopsd-xc Raised at file xenopsd/xc/xenops_server_xen.ml, line 834 Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] 2/3 xenopsd-xc Called from file xenopsd/xc/xenops_server_xen.ml, line 930 Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] 3/3 xenopsd-xc Called from file xapi-idl/lib/debug.ml, line 220 Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||backtrace] Sep 1 13:47:07 localhost xenopsd-xc: [error||44 ||xenops_interface] Xenctrlext.Unix_error(57, "105: No buffer space available") (File "xapi-idl/xen/xenops_interface.ml", li ne 168, characters 51-58) Sep 1 13:47:07 localhost xapi: [error||0 |dbsync (update_env) R:531918011f4b|xenops_interface] Xenops_interface.Xenopsd_error([S(Internal_error);S(Xenctrlext.Unix_error(57, "105: No buffer space available"))]) (File "xen/xenops_interface.ml", line 160, characters 49-56) ``` Signed-off-by: Edwin Török <[email protected]>
1 parent 5db345e commit 1d00c36

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

lib/debug.ml

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -191,19 +191,34 @@ let rec split_c c str =
191191
:: split_c c (String.sub str (i + 1) (String.length str - i - 1))
192192
with Not_found -> [str]
193193

194-
let log_backtrace_exn ?(level = Syslog.Err) ?(msg = "error") exn _bt =
195-
Backtrace.is_important exn ;
196-
let all = split_c '\n' Backtrace.(to_string_hum (remove exn)) in
194+
let log_backtrace_exn ?(level = Syslog.Err) ?(msg = "error") exn bt =
195+
(* We already got the backtrace in the `bt` argument when called from with_thread_associated.
196+
Log that, and remove `exn` from the backtraces table.
197+
If with_backtraces was not nested then looking at `bt` is the only way to get
198+
a proper backtrace, otherwise exiting from `with_backtraces` would've removed the backtrace
199+
from the thread-local backtraces table, and we'd always just log a message complaining about
200+
with_backtraces not being called, which is not true because it was.
201+
*)
202+
let bt' = Backtrace.remove exn in
203+
(* bt could be empty, but bt' would contain a non-empty warning, so compare 'bt' here *)
204+
let bt = if bt = Backtrace.empty then bt' else bt in
205+
let all = split_c '\n' Backtrace.(to_string_hum bt) in
197206
(* Write to the log line at a time *)
198207
output_log "backtrace" level msg
199208
(Printf.sprintf "Raised %s" (Printexc.to_string exn)) ;
200209
List.iter (output_log "backtrace" level msg) all
201210

202-
let log_backtrace e bt = log_backtrace_exn e bt
211+
let log_backtrace_internal ?level ?msg e _bt =
212+
Backtrace.is_important e;
213+
log_backtrace_exn ?level ?msg e (Backtrace.remove e)
214+
215+
let log_backtrace e bt = log_backtrace_internal e bt
203216

204217
let with_thread_associated task f x =
205218
ThreadLocalTable.add tasks task ;
206-
let result = Backtrace.with_backtraces (fun () -> f x) in
219+
let result = Backtrace.with_backtraces (fun () ->
220+
try f x
221+
with e -> Backtrace.is_important e; raise e) in
207222
ThreadLocalTable.remove tasks ;
208223
match result with
209224
| `Ok result ->
@@ -214,7 +229,7 @@ let with_thread_associated task f x =
214229
output_log "backtrace" Syslog.Err "error"
215230
(Printf.sprintf "%s failed with exception %s" task
216231
(Printexc.to_string exn)) ;
217-
log_backtrace exn bt ;
232+
log_backtrace_exn exn bt ;
218233
raise exn
219234

220235
let with_thread_named name f x =
@@ -299,5 +314,5 @@ functor
299314

300315
let log_and_ignore_exn f =
301316
try f ()
302-
with e -> log_backtrace_exn ~level:Syslog.Debug ~msg:"debug" e ()
317+
with e -> log_backtrace_internal ~level:Syslog.Debug ~msg:"debug" e ()
303318
end

0 commit comments

Comments
 (0)