-
Notifications
You must be signed in to change notification settings - Fork 44
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
Segfault in lwt_echo_post #139
Comments
Hm, last build was using some weird environment. I've ran dune clean to get rid of old artifacts and rebuild from scratch. Backtrace changed a bit:
Curl first uploads 3620M (out of 7.2G file), then upload stops and it proceeds to downloading 3620M back. Once downloaded server process receives segfault.
It points to this part of the code of faraday.ml in my _opam directory: 369 let rec shift_buffers t written =
370 try
371 let { len; _ } as iovec = Buffers.dequeue_exn t.scheduled in
372 if len <= written then begin
373 shift_buffers t (written - len)
374 end else
375 Buffers.enqueue_front (IOVec.shift iovec written) t.scheduled
376 with Dequeue_empty ->
377 assert (written = 0);
378 if t.scheduled_pos = t.write_pos then begin
379 t.scheduled_pos <- 0;
380 t.write_pos <- 0
381 end |
FWIW some top-level interaction: # let rec shift_buffers t written =
try
let len = 42 in
if len <= written then begin
(shift_buffers[@tailcall]) t (written - len)
end else
Printf.printf "bla\n"
with _ ->
assert (written = 0)
;;
Warning 51: expected tailcall # let rec shift_buffers t written =
(* try *)
let len = 42 in
if len <= written then begin
(shift_buffers[@tailcall]) t (written - len)
end else
Printf.printf "bla\n"
(* with _ ->
assert (written = 0) *);;
val shift_buffers : 'a -> int -> unit = <fun> ~> calling ourselves within the exception handler leads to stack allocation (of the exception handler AFAICT), you'll have to move the recursive self-call out of the exception handler (the tailcall annotation is pretty good at spotting this) |
Cool! For some reason I was trying to add tailcall annotation to pinned faraday repo clone on my laptop and make dune stop on this warning, without any results. Should've tried with top level :) Actually I've suggested to get rid of that exception altogether in faraday/51, but for other reasons. |
(another option is to have the |
Just rebuilt with my fork of faraday and it does not segfault any more, although odd behavior with buffering of 3+GB in memory is still in place. |
Excessive buffering was due to asynchronous write in example echo handler. Can be fixed like this: --- a/examples/lib/httpaf_examples.ml
+++ b/examples/lib/httpaf_examples.ml
@@ -39,8 +39,9 @@ module Server = struct
let request_body = Reqd.request_body reqd in
let response_body = Reqd.respond_with_streaming reqd response in
let rec on_read buffer ~off ~len =
- Body.write_bigstring response_body buffer ~off ~len;
- Body.schedule_read request_body ~on_eof ~on_read;
+ Body.schedule_bigstring response_body buffer ~off ~len;
+ Body.flush response_body (fun () ->
+ Body.schedule_read request_body ~on_eof ~on_read);
and on_eof () =
Body.close_writer response_body
in |
Might be related with #64, but I'm experiencing segfault in lwt_echo_post compiled to native binary.
Steps to reproduce:
Observed behavior:
Probably process ran out of stack. Is shift_buffers function expected to be subject to tailcall optimization, or the issue is due to excessive buffering?
The text was updated successfully, but these errors were encountered: