uNetworking / uWebSockets.js

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

Dedicated compression with publish, set to don't compress is broken #426

Closed lhkzh closed 3 years ago

lhkzh commented 3 years ago

when client send, server publish then shutdown. server run windows10, node 14.5.0

server code

const UWS=require('./uWebSockets.js/uws');
const app=UWS.App({})
    .ws('/*', {
    idleTimeout: 30,
    maxBackpressure: 1024,
    maxPayloadLength: 512,
    compression: UWS.DEDICATED_COMPRESSOR_3KB,
    message: (ws, message, isBinary) => {
        let ok = ws.send(message, isBinary, true);
        ws.publish("hh",message,isBinary);//pub
    },
        open:(ws)=>{
        ws._=Date.now();
            console.log("on_ws_open")
            ws.subscribe("hh");// sub
        },
    close:(ws)=>{
        console.log("on_ws_close",Date.now()-ws._)
    }
}).get('/*', (res, req) => {
    res.writeStatus('200 OK').writeHeader('IsExample', 'Yes').end('Hello there!');
}).listen(9001, (listenSocket) => { if (listenSocket) {console.log('Listening to port 9001'); }});

client code,run in chrome

var con=new WebSocket("ws://127.0.0.1:9001");
con.ts=Date.now();con.onmessage=e=>{console.log(e.data)};
con.onclose=e=>{console.log("on_lost",e.code,e.reason,Date.now()-con.ts)};
con.onopen=e=>{ setInterval(()=>{ if(con.readyState!=3)con.send("abc:"+Date.now()) },1000) }
hst-m commented 3 years ago

I see that changing the ws.publish("hh",message,isBinary) line to ws.publish("hh",message,isBinary,true) fixes the issue, something with not setting the compression parameter to true causing problem Segmentation fault (core dumped) https://github.com/uNetworking/uWebSockets/blob/master/src/WebSocketContextData.h#L213

it helps to simplify the app to what still causes issue:

uws=require('uWebSockets.js')
uws.App().ws('/',{
    compression:uws.DEDICATED_COMPRESSOR_3KB, // any dedicated compressor
    open:ws=>{ws.subscribe('1');ws.publish('1','a')} // not setting compression parameter true = seg fault
}).listen(3000,()=>{})
new WebSocket("ws://localhost:3000")
lhkzh commented 3 years ago

yes.

ghost commented 3 years ago

What happens if you specify false?

hst-m commented 3 years ago

false also throws error, must specify true to work

lhkzh commented 3 years ago

What happens if you specify false?

ws.publish('1','a'); core dump crash ws.publish('1','a',true); core dump crash ws.publish('1','a',true,false); core dump crash ws.publish('1','a',false,false); core dump crash ws.publish('1','a',true,true); success (need client support comporess ws.publish('1','a',false,true); success (need client support comporess

ghost commented 3 years ago

I can reproduce, not for SHARED_COMPRESSOR but with DEDICATED_COMPRESSOR. Something is wrong

ghost commented 3 years ago

Okay I see the issue - it's an old flaw that has never worked (and has not been caught in testing). You can work-around it by setting compress = true always, or use SHARED_COMPRESSOR. Will fix

ghost commented 3 years ago

yarn add uWebSockets.js@uNetworking/uWebSockets.js#v18.13.0