uNetworking / uWebSockets.js

μWebSockets for Node.js back-ends :metal:
Apache License 2.0
8.08k stars 574 forks source link

Video streaming example segfaults #736

Closed o5k closed 2 years ago

o5k commented 2 years ago

While going off of the VideoStreamer example to implement reading from a read stream, I'm hitting into a segfault. The example code alone is enough to cause this, so you should be able to repro on that alone. I've noticed that adding console.logs in a bunch of places (to try and figure out which call to any uWS function causes it, no luck) made the issue less common. Some kind of race condition perhaps...?

Is the VideoStreamer example still correct, or should reading from a large stream be implemented differently?

Exception thrown at 0x00000001402068F2 in node.exe: 0xC0000005: Access violation reading location 0x0000005F656C6309.

2022-04-26_20-45-43

I'm running Node v16.14.2 on Windows, and uWS.js 20.8.0.

zdm commented 2 years ago

I always said that for me it crashes pnly when chunked encoding used (write + onWrite) and only under linux.

zdm commented 2 years ago

@alexhultman Is it possible to build v20.6.0 for node 18 or code requires update for node 18 api changes?

ghost commented 2 years ago

I don't know and I have no time to find out. I need a snippet that reproduces the segfault, without involving complex use of nodejs streams. Give me that and I can look at it.

zdm commented 2 years ago

I already provided a snipper, pls, see above. https://github.com/uNetworking/uWebSockets.js/issues/736#issuecomment-1112608247

The problem, that it crashes not for everybody. For me under my my linux environment - it crashes usually between 200 and 500 requests. But it not crashes for some other guys.

o5k commented 2 years ago

I already provided a snipper, pls, see above.

The point is trying to reproduce it without read streams, haven't been able to yet myself, but I've also had little time to test. That said, even then, an incompatibility between read streams and uWS.js would be a problem. uWS.js should be able to serve larger content without first loading the entire file in memory (although, considering this has gone unreported, I suppose most people don't use it this way)... perhaps other ways to read a stream would be a quick fix for the example?

I feel like uWS.js should be agnostic to the source of the buffer passed to it. Only if we're certain this is a Node/v8 issue this would be passed on to them

ghost commented 2 years ago

Obviously it should work but to eliminate all third party, and to allow me to test the C++ core I need a reproducer that triggers using only uWS. I cannot debug a complex app that segfaults in some cases, but gives nothing for others.

zdm commented 2 years ago

My snippet is pure uws, without any additional code. Dows it crashes for you? https://github.com/uNetworking/uWebSockets.js/issues/736#issuecomment-1112608247

ghost commented 2 years ago

No I posted a screenshot of me running it until 8000 and no issues

zdm commented 2 years ago

Pls, run it several times. It can not crush at all during one run.

ghost commented 2 years ago

@zdm

Can you run your snippet, but wrap stream.resume(); in try catch. So that if stream.resume(); throws, it still returns true

zdm commented 2 years ago

yes. will try i am working on reliable repro. code for you

zdm commented 2 years ago

Try / catch for stream.resume() didn't helped. Didn't caught anything, just segfault.

zdm commented 2 years ago

The problem is definitely in the streaming code, when I read file to the memory and send with the single end() call it is not crashes.

Now it crashes only when I request files using the browser. Maybe the problem is in the multiple persistent connections, that browser opens and reuse.

zdm commented 2 years ago

Finally, I found, where is the problem. It crashes, when tryEnd inside onWritable returns done = true. I am preparing code snippet.

ghost commented 2 years ago

Sounds good. Trying to hit that case in fuzzing..

zdm commented 2 years ago

Yes, it is 100% crashes at this place, but it is gard to create repro code, hard to choose initial buffer size to emulate this case.

zdm commented 2 years ago

Finally I found correct buffer size for ubuntu, For windows it will not work, need other size.

#!/usr/bin/env node

import http from "http";
import uws from "uWebSockets.js";

const app = uws.App().get( "/*", ( res, req ) => {
    res.onAborted( () => {} );

    var b = Buffer.from( "a".repeat( 2_588_610 + 100 ) ),
        size = b.length,
        ok,
        done;

    console.log( "--- create backpressure" );

    // create backpressure
    while ( 1 ) {
        [ok, done] = res.tryEnd( b, size );
        console.log( "ok:", ok, ", done:", done );

        if ( !ok ) break;
    }

    console.log( "--- backpressure created" );

    res.onWritable( offset => {
        console.log( "--- on writable", offset, size - offset );

        const chunk = b.slice( offset, size );

        // const chunk = b.buffer.slice( b.byteOffset, b.byteOffset + b.byteLength );

        console.log( offset + chunk.length, size );

        [ok, done] = res.tryEnd( chunk, size );
        console.log( "ok:", ok, ", done:", done );

        return ok;
    } );
} );

await new Promise( resolve => app.listen( 80, resolve ) );

await new Promise( resolve => {
    http.request( "http://127.0.0.1/", {}, res => {
        res.resume();
        res.once( "end", resolve );
    } ).end();
} );
zdm commented 2 years ago

Outout:

--- create backpressure
ok: false , done: false
--- backpressure created
--- on writable 2588611 99
2588710 2588710
ok: true , done: true
Segmentation fault (core dumped)
ghost commented 2 years ago

It makes sense.

Alright, very good reporting and testing!

ghost commented 2 years ago

Here is an asan report of the trigger @zdm made:

================================================================= ==8172==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000028f38 at pc 0x7f35a0e0acdb bp 0x7ffdca2c5c10 sp 0x7ffdca2c5c00 READ of size 8 at 0x602000028f38 thread T0

0 0x7f35a0e0acda (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x15acda)

#1 0x7f35a0de3078 in uWS::HttpContext<false>::init()::{lambda(us_socket_t*)#1}::__invoke(us_socket_t*) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x133078)
#2 0x7f35a0e1488a in us_internal_dispatch_ready_poll (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x16488a)
#3 0x164bcd3 in uv__io_poll ../deps/uv/src/unix/epoll.c:374
#4 0x1639ee7 in uv_run ../deps/uv/src/unix/core.c:389
#5 0xa95e44 in node::SpinEventLoop(node::Environment*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95e44)
#6 0xb9b045 in node::NodeMainInstance::Run() (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xb9b045)
#7 0xb1bbc2 in node::Start(int, char**) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xb1bbc2)
#8 0x7f38293d90b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
#9 0xa93e8d in _start (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa93e8d)

0x602000028f38 is located 8 bytes inside of 16-byte region [0x602000028f30,0x602000028f40) freed by thread T0 here:

0 0x7f3829a358df in operator delete(void*) (/usr/lib/gcc/x86_64-linux-gnu/9/libasan.so+0x1108df)

#1 0x7f35a0dd3671  (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x123671)
#2 0x7f35a0dd2d04  (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x122d04)
#3 0x7f35a0e02cdd in void HttpResponseWrapper::res_tryEnd<false>(v8::FunctionCallbackInfo<v8::Value> const&) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x152cdd)
#4 0xda4a6f in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xda4a6f)
#5 0xda600e in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xda600e)
#6 0x16d8638 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x16d8638)
#7 0x165a98e in Builtins_InterpreterEntryTrampoline (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x165a98e)
#8 0x1658a9b in Builtins_JSEntryTrampoline (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x1658a9b)
#9 0x16587c2 in Builtins_JSEntry (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x16587c2)
#10 0xe88a1e in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xe88a1e)
#11 0xe89b1e in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xe89b1e)
#12 0xd61e87 in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xd61e87)
#13 0xa95294 in node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95294)
#14 0xa95477 in node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95477)
#15 0x7f35a0e0ab91  (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x15ab91)
#16 0x7f35a0de3078 in uWS::HttpContext<false>::init()::{lambda(us_socket_t*)#1}::__invoke(us_socket_t*) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x133078)
#17 0x164bcd3 in uv__io_poll ../deps/uv/src/unix/epoll.c:374
#18 0x1639ee7 in uv_run ../deps/uv/src/unix/core.c:389
#19 0xa95e44 in node::SpinEventLoop(node::Environment*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95e44)
#20 0xb9b045 in node::NodeMainInstance::Run() (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xb9b045)
#21 0xb1bbc2 in node::Start(int, char**) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xb1bbc2)
#22 0x7f38293d90b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

previously allocated by thread T0 here:

0 0x7f3829a34947 in operator new(unsigned long) (/usr/lib/gcc/x86_64-linux-gnu/9/libasan.so+0x10f947)

#1 0x7f35a0e04d55 in void HttpResponseWrapper::res_onWritable<false>(v8::FunctionCallbackInfo<v8::Value> const&) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x154d55)
#2 0xda4a6f in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xda4a6f)
#3 0xda600e in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xda600e)
#4 0x16d8638 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x16d8638)
#5 0x165a98e in Builtins_InterpreterEntryTrampoline (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x165a98e)
#6 0x1658a9b in Builtins_JSEntryTrampoline (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x1658a9b)
#7 0x16587c2 in Builtins_JSEntry (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0x16587c2)
#8 0xe88a1e in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xe88a1e)
#9 0xe89b1e in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xe89b1e)
#10 0xd61e87 in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xd61e87)
#11 0xa95294 in node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95294)
#12 0xa95477 in node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/home/alexhultman/uWebSockets.js/node-v18.0.0-linux-x64/bin/node+0xa95477)
#13 0x7f35a0dde34b  (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x12e34b)
#14 0x7f35a0dccf2e in ofats::any_detail::handler_traits<bool, uWS::HttpRouter<uWS::HttpContextData<false>::RouterData>*>::large_handler<uWS::HttpContext<false>::onHttp(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ofats::any_invocable<void (uWS::HttpResponse<false>*, uWS::HttpRequest*)>&&, bool)::{lambda(auto:1*)#1}>::call(ofats::any_detail::storage&, uWS::HttpRouter<uWS::HttpContextData<false>::RouterData>*) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x11cf2e)
#15 0x7f35a0de4828  (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x134828)
#16 0x7f35a0de3db6 in ofats::any_detail::handler_traits<void*, void*, uWS::HttpRequest*>::small_handler<uWS::HttpContext<false>::init()::{lambda(us_socket_t*, char*, int)#1}::operator()(us_socket_t*, char*, int) const::{lambda(void*, uWS::HttpRequest*)#1}>::call(ofats::any_detail::storage&, void*, uWS::HttpRequest*) (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x133db6)

SUMMARY: AddressSanitizer: heap-use-after-free (/home/alexhultman/uWebSockets.js/dist/uws_linux_x64_108.node+0x15acda) Shadow bytes around the buggy address: 0x0c047fffd190: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fa 0x0c047fffd1a0: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fa 0x0c047fffd1b0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa 0x0c047fffd1c0: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd 0x0c047fffd1d0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd =>0x0c047fffd1e0: fa fa fd fa fa fa fd[fd]fa fa fd fa fa fa fd fd 0x0c047fffd1f0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fa 0x0c047fffd200: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa 0x0c047fffd210: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fa 0x0c047fffd220: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa 0x0c047fffd230: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==8172==ABORTING

zdm commented 2 years ago

Ok, to reproduce it under windows other code is required, but I think - reason of segfauls is the same. Tell me if you still need to reproduce it under win.

zdm commented 2 years ago

Interesting, but under windows there is no segfault:

--- create backpressure
{ ok: true, done: false }
{ ok: true, done: false }
{ ok: true, done: false }
{ ok: true, done: false }
{ ok: true, done: false }
{ ok: false, done: false }
--- backpressure created
--- on writable 500000 99500000
tryEnd call inside onWritable callback: { ok: true, done: true }
--- REQUEST DONE
zdm commented 2 years ago

I would like to ask 2 questions, not related to this issue:

  1. end() and tryEnd() accepts RecognizedString, that should be string | ArrayBuffer | Uint8Array | Int8Array | Uint16Array | Int16Array | Uint32Array | Int32Array | Float32Array | Float64Array. But I see, that it also works well with the nodejs Buffer. Should I convert Buffer to ArrayBuffer before pass it to the end() call or it is safe to use Buffer directly?

  2. end() method has close argument, but tryEnd() - not. I think it should be added to have possibility to close connection after stream finished.

ghost commented 2 years ago

Fix is here https://github.com/uNetworking/uWebSockets/commit/e4c6fbf8defda2aeaf941e5c656bdc589d7d331c

Will have it run in fuzzing for a while then make a release. Very good reporting and creation of reproducer! Thanks

zdm commented 2 years ago

Thanks

ghost commented 2 years ago

https://github.com/uNetworking/uWebSockets/releases/tag/v20.14.0

yolo

ghost commented 2 years ago

Can you try this https://github.com/uNetworking/uWebSockets.js/releases/tag/v20.9.0

zdm commented 2 years ago

Works without errors, thank you.

o5k commented 2 years ago

Can confirm it works like a charm, thank you very much!