tg123 / websockify-nginx-module

Embed websockify into Nginx (convert any tcp connection into websocket)
MIT License
141 stars 60 forks source link

Closing connection on close request by opcode #2

Closed Azrah closed 9 years ago

tg123 commented 10 years ago

thanks

please use 4 spaces instead of tab

tg123 commented 10 years ago

we cant just close the upstream tcp connection that way there is a lot work to do

please refer to http://tools.ietf.org/html/rfc6455#section-5.5.1 and http://tools.ietf.org/html/rfc6455#section-7

tg123 commented 10 years ago

er... let me explain why this mod is now doing nothing when it received the close frame

typically, server should send the echo close frame and close the connection after it received the close frame here the connection means tcp connection from CLIENT to NGINX

well, at this time, we have not implement the echo close frame and CLIENT may fire a TCP CLOSE to NGINX after some time the NGINX did not close the connection. NGINX would call close the whole connection including the upstream connection (let say connection from NGINX to VNC Server) when CLIENT close the connection

if we call close upstream connection here the upstream connection might be closed twice and core the worker

Azrah commented 10 years ago

Ok, I wasn't aware of all the details in the RFC. I will try to echo the close frame. In my case I'm not using a VNC connection. I implemented my own server based on a websocket library and disconnecting from my webbased websocket after sending a websocket close takes around 5 minutes, which I really don't like. Just closing the connection seemed to work just fine for me ;)

Azrah commented 10 years ago

I tried implementing a proper closing mechanism, but still have a problem. Can you please check and comment my latest change?

tg123 commented 10 years ago

what s problem

i think you should use something like finalize_request

Azrah commented 10 years ago

Thank you for your input. It is much appreciated. Also it seems to be working with ngx_http_finalize_request(r, NGX_DONE); ngx_http_free_request(r, NGX_DONE); I hope this works in your application as well.

tg123 commented 10 years ago

thanks a lot

i will do some test and merge your code

tg123 commented 10 years ago

connect and disconnet continually will core the worker

nginx 1.5.9

* thread #1: tid = 0x6e80, 0x0000000100014a1b nginx`ngx_rbtree_insert_timer_value(temp=0x0000000000000000, node=0x0000000101869910, sentinel=0x000000010016f5f8) + 27 at ngx_rbtree.c:139, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100014a1b nginx`ngx_rbtree_insert_timer_value(temp=0x0000000000000000, node=0x0000000101869910, sentinel=0x000000010016f5f8) + 27 at ngx_rbtree.c:139
    frame #1: 0x000000010001458a nginx`ngx_rbtree_insert(tree=0x0000000100161178, node=0x0000000101869910) + 138 at ngx_rbtree.c:45
    frame #2: 0x0000000100079a48 nginx`ngx_event_add_timer(ev=0x00000001018698e0, timer=60000) + 376 at ngx_event_timer.h:94
    frame #3: 0x00000001000785d1 nginx`ngx_http_upstream_connect(r=0x0000000102809450, u=0x000000010280a420) + 1633 at ngx_http_upstream.c:1316
    frame #4: 0x0000000100075f5a nginx`ngx_http_upstream_init_request(r=0x0000000102809450) + 2394 at ngx_http_upstream.c:679
    frame #5: 0x000000010007552b nginx`ngx_http_upstream_init(r=0x0000000102809450) + 315 at ngx_http_upstream.c:473
    frame #6: 0x0000000100066fb8 nginx`ngx_http_read_client_request_body(r=0x0000000102809450, post_handler=0x00000001000753f0) + 344 at ngx_http_request_body.c:84
    frame #7: 0x00000001000cede4 nginx`ngx_http_websockify_handler(r=0x0000000102809450) + 1220 at ngx_http_websockify_module.c:700
    frame #8: 0x00000001000487f0 nginx`ngx_http_core_content_phase(r=0x0000000102809450, ph=0x0000000102803b48) + 80 at ngx_http_core_module.c:1410
    frame #9: 0x0000000100045d33 nginx`ngx_http_core_run_phases(r=0x0000000102809450) + 147 at ngx_http_core_module.c:888
    frame #10: 0x0000000100045c93 nginx`ngx_http_handler(r=0x0000000102809450) + 675 at ngx_http_core_module.c:871
    frame #11: 0x0000000100055fda nginx`ngx_http_process_request(r=0x0000000102809450) + 122 at ngx_http_request.c:1852
    frame #12: 0x0000000100059e3a nginx`ngx_http_process_request_headers(rev=0x0000000101862870) + 1898 at ngx_http_request.c:1283
    frame #13: 0x0000000100058c21 nginx`ngx_http_process_request_line(rev=0x0000000101862870) + 1201 at ngx_http_request.c:964
    frame #14: 0x0000000100053eaa nginx`ngx_http_wait_request_handler(rev=0x0000000101862870) + 970 at ngx_http_request.c:486
    frame #15: 0x000000010003eb6d nginx`ngx_kqueue_process_events(cycle=0x0000000101800050, timer=46464, flags=1) + 2125 at ngx_kqueue_module.c:684
    frame #16: 0x000000010002d20f nginx`ngx_process_events_and_timers(cycle=0x0000000101800050) + 303 at ngx_event.c:248
    frame #17: 0x000000010003bd3f nginx`ngx_single_process_cycle(cycle=0x0000000101800050) + 271 at ngx_process_cycle.c:315
    frame #18: 0x00000001000013b1 nginx`main(argc=1, argv=0x00007fff5fbff2a8) + 1473 at nginx.c:404
Azrah commented 10 years ago

Thank you for your feedback. I checked a few things and came to the conclusion to change the result code from NGX_DONE to NGX_OK. If this fixes it, I'm happy. If not, can you please tell me about your test setup, so I can try to reproduce the error on my side.

tg123 commented 9 years ago

ping and close is supported now. close frame will be replied to client.

I tested on firefox. it could display the close reason. image

but chrome still shows timeout. seems a bug within javascript.

however, websockify should not call finalize_request now as it will cause double free.