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

Infinite for loop when upstream partially failing #20

Closed
nzt4567 opened this issue Mar 16, 2016 · 5 comments
Closed

Infinite for loop when upstream partially failing #20

nzt4567 opened this issue Mar 16, 2016 · 5 comments

Comments

@nzt4567
Copy link

nzt4567 commented Mar 16, 2016

Hi,

when the network is not 100% reliable and part of the response from upstream is lost then the loop in ngx_http_brotli_body_filter seems to be infinite. This happened when the connection to upstream was successful, the upstream returned some data but then it stopped responding and nginx's proxy_read_timeout has run out.

According to gdb ngx_http_brotli_filter_add_data returns NGX_OK and then ngx_http_brotli_filter_process returns NGX_AGAIN. This consumes all the resources dedicated to given worker process and stays like this forever. Stracing such process shows no system calls, which supports the idea of infinite loop in user space. Below are few lines from gdb when breakpoint was set at ngx_http_brotli_body_filter and then the code was observed using next/nexti/step/stepi/finish & bt gdb commands.

`
(gdb) bt
#0 ngx_http_brotli_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at ../modules/ngx_brotli-module/src/ngx_http_brotli_filter_module.cc:324
#1 0x00000000004a5e0f in ngx_http_postpone_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/ngx_http_postpone_filter_module.c:82
#2 0x00000000004a6448 in ngx_http_ssi_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/modules/ngx_http_ssi_filter_module.c:411
#3 0x00000000004ac609 in ngx_http_charset_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/modules/ngx_http_charset_filter_module.c:557
#4 0x00000000004e36c3 in ngx_http_mp4_body_filter (req=0x1fbfd70, in=0x7fffd1099500) at ../modules/ngx_mp4_filter_module/ngx_http_mp4_filter_module.c:190
#5 0x000000000054572d in ngx_http_lua_capture_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at ../modules/lua-nginx-module/src/ngx_http_lua_capturefilter.c:133
#6 0x000000000042876a in ngx_output_chain (ctx=0x197abb0, in=0x7fffd1099500) at src/core/ngx_output_chain.c:74
#7 0x00000000004b1684 in ngx_http_copy_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/ngx_http_copy_filter_module.c:152
#8 0x00000000004a33e8 in ngx_http_range_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/modules/ngx_http_range_filter_module.c:623
#9 0x00000000004675e8 in ngx_http_output_filter (r=0x1fbfd70, in=0x7fffd1099500) at src/http/ngx_http_core_module.c:1978
#10 0x00000000004765ef in ngx_http_send_special (r=0x1fbfd70, flags=2) at src/http/ngx_http_request.c:3346
#11 0x000000000048fbf8 in ngx_http_upstream_finalize_request (r=0x1fbfd70, u=0x315b868, rc=-1) at src/http/ngx_http_upstream.c:4107
#12 0x000000000048ef9a in ngx_http_upstream_process_request (r=0x1fbfd70, u=0x315b868) at src/http/ngx_http_upstream.c:3704
#13 0x000000000048ecbb in ngx_http_upstream_process_upstream (r=0x1fbfd70, u=0x315b868) at src/http/ngx_http_upstream.c:3628
#14 0x0000000000489865 in ngx_http_upstream_handler (ev=0x7f69659cf250) at src/http/ngx_http_upstream.c:1111
#15 0x000000000044c383 in ngx_event_expire_timers () at src/event/ngx_event_timer.c:94
#16 0x000000000044a21e in ngx_process_events_and_timers (cycle=0x18ee660) at src/event/ngx_event.c:272
#17 0x00000000004550ef in ngx_worker_process_cycle (cycle=0x18ee660, data=0x0) at src/os/unix/ngx_process_cycle.c:765
#18 0x0000000000451f71 in ngx_spawn_process (cycle=0x18ee660, proc=0x455027 <ngx_worker_process_cycle>, data=0x0, name=0x5a49b8 "worker process", respawn=-3)

at src/os/unix/ngx_process.c:202

#19 0x00000000004542aa in ngx_start_worker_processes (cycle=0x18ee660, n=1, type=-3) at src/os/unix/ngx_process_cycle.c:366
#20 0x0000000000453a14 in ngx_master_process_cycle (cycle=0x18ee660) at src/os/unix/ngx_process_cycle.c:138
#21 0x000000000042140c in main (argc=1, argv=0x7fffd1099c08) at src/core/nginx.c:367

`

`
#0 ngx_http_brotli_body_filter (r=0x1fbfd70, in=0x7fffd1099500) at ../modules/ngx_brotli-module/src/ngx_http_brotli_filter_module.cc:324

    rc = 0
    flush = 0
    cl = 0x197ab20
    ctx = 0x197a610

`

(gdb) print *in
$14 = {buf = 0x197ab60, next = 0x0}
(gdb) print *in->buf
$16 = {pos = 0x0, last = 0x0, file_pos = 0, file_last = 0, start = 0x0, end = 0x0, tag = 0x0, file = 0x0, shadow = 0x0, temporary = 0, memory = 0, mmap = 0, recycled = 0,
in_file = 0, flush = 1, sync = 0, last_buf = 0, last_in_chain = 0, last_shadow = 0, temp_file = 0, num = 0}

Code of libbrotli/brotli/ngx_brotli is current as of today and we are able to replicate this w/o any problem so if you need anything else to debug this, just let me know.

Thanks,
Tomas

@PiotrSikora
Copy link
Contributor

Thank you for the detailed report! I was able to reproduce the issue and I'm looking into it now.

@nzt4567
Copy link
Author

nzt4567 commented Mar 17, 2016

You are welcome ;) Let me know when you want to test it or need anything else.

Thanks,
Tomas

PiotrSikora added a commit to PiotrSikora/ngx_brotli that referenced this issue Mar 18, 2016
Output filters receive only special buffer with "flush" flag set
in case when upstream sent headers, but timed out before sending
request body.

Previously, this case wasn't handled correctly, because of the wrong
assumption that Brotli compressor always produces output when called
with "flush" parameter, which isn't the case if there is no input to
compress.

Reported by Tomas Kvasnicka on GitHub (issue google#20).

Change-Id: I13c059d41ef1fd792f2a56a390c0417ec6b611ab
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor

Can you check if this patch works for you? Thanks!

@nzt4567
Copy link
Author

nzt4567 commented Mar 18, 2016

Piotr, thanks a lot! :) I can confirm this is working so brotli and nginx are friends again 👍

Once again, thanks again for your quick reaction and fix!

@nzt4567 nzt4567 closed this as completed Mar 18, 2016
@PiotrSikora
Copy link
Contributor

Thanks for testing! I'll merge it once it passes internal review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants