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

Handle ERROR_MORE_DATA in reading from Windows named pipe #1772

Closed
wants to merge 1 commit into from

Conversation

Thomasdezeeuw
Copy link
Collaborator

@Thomasdezeeuw
Copy link
Collaborator Author

I don't really have a way to test this. I don't know how to create the "error" conditions such that it trigger this code branch. It was manually tested by @MolotovCherry, see tokio-rs/tokio#6460 (comment).

@MolotovCherry
Copy link

MolotovCherry commented Apr 5, 2024

Just for transparency, here's my testing code below. On the original issue, one can find the original failing case.

This code below would hit the first unreachable!() before this patch was made. Now it responds Ok with the received buffer data

Code
use tokio::{
    io::{AsyncReadExt as _, AsyncWriteExt},
    net::windows::named_pipe::{ClientOptions, ServerOptions},
    task,
};

#[tokio::main]
async fn main() {
    const BUFFER_SIZE: usize = 4096;

    const PIPE_NAME: &str = r"\\.\pipe\foobar";

    let h = task::spawn(async {
        let mut buf = vec![0; 4096];

        let mut server = ServerOptions::new()
            .access_inbound(true)
            .access_outbound(true)
            .reject_remote_clients(true)
            .pipe_mode(tokio::net::windows::named_pipe::PipeMode::Message)
            .in_buffer_size(BUFFER_SIZE as u32)
            .out_buffer_size(BUFFER_SIZE as u32)
            .create(PIPE_NAME)
            .unwrap();

        server.connect().await;

        let Ok(size) = server.read(&mut buf).await else {
            // ERROR_MORE_DATA hit
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );

        let Ok(size) = server.read(&mut buf).await else {
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );
    });

    let c = task::spawn(async {
        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
        let mut client = ClientOptions::new().open(PIPE_NAME).unwrap();
        // 4096 + 10 byte long message. Since the max buffer is 4096, this triggers windows ERROR_MORE_DATA
        // in pipe message mode once it passes the limit. And that "error" was the reason for the failure
        client.write_all(b"osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt_overwrite").await.unwrap();
    });

    h.await.unwrap();
    c.await.unwrap();
}

@Darksonn
Copy link
Contributor

Darksonn commented Apr 6, 2024

Does this actually behave correctly for large buffers with pipe mode message? Let's say we passed a larger buffer to read, then mio would still only use 4kb internally for the operation, but with pipe mode messages, the reads are supposed to correspond to individual messages.

Or maybe it's okay to split up messages in multiple reads because of the ERROR_MORE_DATA error.

@MolotovCherry
Copy link

MolotovCherry commented Apr 6, 2024

Does this actually behave correctly for large buffers with pipe mode message? Let's say we passed a larger buffer to read, then mio would still only use 4kb internally for the operation, but with pipe mode messages, the reads are supposed to correspond to individual messages.

Or maybe it's okay to split up messages in multiple reads because of the ERROR_MORE_DATA error.

I set the in/out buffer to 8k, and also the read buffer as well. I still receive the same result as normal on a basic test. So as far as I know a larger buffer works normally.
Edit: What I mean is, it reads in 4096 chunks regardless of the size of the buffer

However, I was able to cause tokio to go into unreachable code with the following:

Code
use tokio::{
    io::{AsyncReadExt as _, AsyncWriteExt},
    net::windows::named_pipe::{ClientOptions, ServerOptions},
    task,
};

#[tokio::main]
async fn main() {
    const BUFFER_SIZE: usize = 4096 * 2;

    const PIPE_NAME: &str = r"\\.\pipe\foobar";

    let h = task::spawn(async {
        let mut buf = vec![0; 4096 * 2];

        let mut server = ServerOptions::new()
            .access_inbound(true)
            .access_outbound(true)
            .reject_remote_clients(true)
            .pipe_mode(tokio::net::windows::named_pipe::PipeMode::Message)
            .in_buffer_size(BUFFER_SIZE as u32)
            .out_buffer_size(BUFFER_SIZE as u32)
            .create(PIPE_NAME)
            .unwrap();

        server.connect().await;

        let Ok(size) = server.read(&mut buf).await else {
            // ERROR_MORE_DATA hit
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );

        let Ok(size) = server.read(&mut buf).await else {
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );

        let Ok(size) = server.read(&mut buf).await else {
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );
        let Ok(size) = server.read(&mut buf).await else {
            // ERROR_MORE_DATA hit
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );
        let Ok(size) = server.read(&mut buf).await else {
            // ERROR_MORE_DATA hit
            unreachable!()
        };

        println!(
            "{size:?}: buf: {:?}",
            std::str::from_utf8(&buf[..size]).unwrap()
        );
    });

    let c = task::spawn(async {
        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
        let mut client = ClientOptions::new().open(PIPE_NAME).unwrap();
        // 4096 + 10 byte long message. Since the max buffer is 4096, this triggers windows ERROR_MORE_DATA
        // in pipe message mode once it passes the limit. And that "error" was the reason for the failure

        // this first one is ok
        client.write_all(b"osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt_overwrite").await.unwrap();
        // this second one causes an unreachable panic. comment it out and the rest will work fine
        client.write_all(b"osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt_overwrite").await.unwrap();
        client.write_all(b"test").await;
    });

    h.await.unwrap();
    c.await.unwrap();
}

Panic message:

    Finished dev [unoptimized + debuginfo] target(s) in 0.51s
     Running `R:\rust\debug\f.exe`
4096: buf: "osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt"
10: buf: "_overwrite"
thread 'tokio-runtime-worker' panicked at mio\src\sys\windows\named_pipe.rs:871:14:
internal error: entered unreachable code
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'worker thread panicking; aborting process
tokio-runtime-worker' panicked at mio\src\sys\windows\named_pipe.rserror: process didn't exit successfully: `R:\rust\debug\f.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)

Source:

_ => unreachable!(),

@Darksonn
Copy link
Contributor

Darksonn commented Apr 8, 2024

Hmm. Perhaps the overlapped callback runs twice with ERROR_MORE_DATA for the follow-up?

@MolotovCherry What is the value of io.read when you hit that unreachable!()?

@MolotovCherry
Copy link

MolotovCherry commented Apr 8, 2024

Hmm. Perhaps the overlapped callback runs twice with ERROR_MORE_DATA for the follow-up?

@MolotovCherry What is the value of io.read when you hit that unreachable!()?

@Darksonn I added a println for io.read right before this line

let mut buf = match mem::replace(&mut io.read, State::None) {

Here are the results:

    Finished dev [unoptimized + debuginfo] target(s) in 3.50s
     Running `R:\rust\debug\f.exe`
Pending([], 0)
4096: buf: "osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt"
Pending([], 0)
10: buf: "_overwrite"
Err(Os { code: 234, kind: Uncategorized, message: "More data is available." })
thread 'tokio-runtime-worker' panicked at mio\src\sys\windows\named_pipe.rs:872:14:
internal error: entered unreachable code
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'worker thread panicking; aborting process
tokio-runtime-workererror: process didn't exit successfully: `R:\rust\debug\f.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)

Edit: I may not have made this part very clear:

Does this actually behave correctly for large buffers with pipe mode message?

If a bigger io.read buffer above 4096 is provided, it reads in only 4096 at a time, regardless of the bigger buffer. So I think the answer is no. (Even when providing a bigger buffer on the pipe itself, I saw the same behavior; only 4096 reads max). I'm guessing this is probably related to tokio-rs/tokio#5307.

My consensus is that the messages past 4096 should not be split up into multiple reads when the read buffer is large enough to hold them, since they should represent only 1 message. That is, if you passed a sufficiently large buffer (8k for example) to the windows read call and wrote 6k, the windows call would work and read 6k into the buffer since there's room (no error_more_data behavior). But if you pass the 8k buffer to tokio and write 6k, it will split it up into 2 reads, 1 4096 read and 1 1904 read. The underlying windows behavior should be mirrored here, but currently isn't because of the 4096 limit. This can lead to single messages > 4096 getting cut into 2 or more despite there being enough read buffer room

@Darksonn
Copy link
Contributor

Darksonn commented Apr 8, 2024

The output you shared probably means that io.read was set to Err(ERROR_MORE_DATA) in schedule_write.

@MolotovCherry
Copy link

MolotovCherry commented Apr 8, 2024

Turns out the unreachable code error was already present in 0.8.11 with the same code in release mode.

It appears here in schedule_read, io.read got set to ERROR_MORE_DATA.

io.read = State::Err(e);

I added an additional arm to the match statement, and all the panics are gone and I now receive the output as normal (minus what I said before about things being broken into 4096 chunks)

Err(ref e) if e.raw_os_error() == Some(ERROR_MORE_DATA as i32) => {
    io.read = State::Pending(buf, 0);
    mem::forget(me.clone());
    true
}

@Thomasdezeeuw
Copy link
Collaborator Author

I've been a little busy, what is left to do here? Add similar code to schedule_read?

Do we want to attempt to read more data straight in the same buffer after if we get ERROR_MORE_DATA, or do we not want to add special code for this?

@Darksonn
Copy link
Contributor

Well, the biggest question mark to me is how to handle messages larger than 4k. Maybe we can handle them split via the ERROR_MORE_DATA code, but I wonder if we might have to enlarge the buffer being used for those cases.

@Thomasdezeeuw
Copy link
Collaborator Author

Well, the biggest question mark to me is how to handle messages larger than 4k. Maybe we can handle them split via the ERROR_MORE_DATA code, but I wonder if we might have to enlarge the buffer being used for those cases.

Making the buffer smaller has also come up before: #1582.

I don't really have an answer as I'm not invested enough, I don't use Windows myself and no one has stepped up and taken charge on this.

@Darksonn
Copy link
Contributor

One option is to just fix the ERROR_MORE_DATA errors and document somewhere that there's a bug when sending messages larger than 4k with pipe message mode. E.g., we can add a note to the Tokio documentation linking to a mio issue about it.

@Thomasdezeeuw
Copy link
Collaborator Author

One option is to just fix the ERROR_MORE_DATA errors and document somewhere that there's a bug when sending messages larger than 4k with pipe message mode. E.g., we can add a note to the Tokio documentation linking to a mio issue about it.

When you say "fix", do you mean split up the reads? Or attempt to get the data into a single buffer?

I assume named pipes are similar to unix pipes in that they're streams, not datagrams, right? Then I think it's perfectly fine we return 4k chunks. We're not dropping data this way.

In another pr/issue we can considering changing buffer size or making this user defined somehow.

@Darksonn
Copy link
Contributor

I assume named pipes are similar to unix pipes in that they're streams, not datagrams, right?

They can be configured to be datagrams. That's what "pipe message mode" refers to.

@Thomasdezeeuw
Copy link
Collaborator Author

They can be configured to be datagrams. That's what "pipe message mode" refers to.

I didn't know (in general I don't know enough about named pipes to make any calls here...).

Then I think, ideally, we attempt to determine the size of the buffer used in the first call to read and use that as the buffer size for Mio's internal buffer. And keep updating that whenever a read call is made. This has the downside that if the user does read(buf[..10]) followed by read(buf[..20]) our buffer sizes will be wrong.

We can also convert any ERROR_MORE_DATA errors into WSAEMSGSIZE (when in datagram/message mode), that would make it consistent with UdpSocket (I think). However I think retrying the read after WSAEMSGSIZE with a larger buffer would resolved the problem. I don't know if that is true for the case we have here.

Overall handling short reads for datagrams is an unsolved problem: rust-lang/rust#55794.

@Darksonn
Copy link
Contributor

I guess the question is whether reading after an ERROR_MORE_DATA gives you the second half of the datagram, or if anything beyond the end is just lost.

@Thomasdezeeuw
Copy link
Collaborator Author

I guess the question is whether reading after an ERROR_MORE_DATA gives you the second half of the datagram, or if anything beyond the end is just lost.

I think Windows returns WSAEMSGSIZE to prevent the lost of data in the case of a short read of a datagram (compared to Unix which just throws the data way). But I don't know if this is true for named pipes as well.

Could really use someone with some Windows expertise on this issue.

@Darksonn
Copy link
Contributor

I don't think we have anyone with windows expertise right now. Let's handle ERROR_MORE_DATA in the two callbacks for now. Fixing the length can be a follow-up PR.

@Darksonn
Copy link
Contributor

We could ask for someone with windows expertise on the Call for Participation section of This Week in Rust.

@Thomasdezeeuw
Copy link
Collaborator Author

We could ask for someone with windows expertise on the Call for Participation section of This Week in Rust.

That sounds like a good idea.

@Thomasdezeeuw Thomasdezeeuw added this to the v1.0 milestone Apr 25, 2024
@dswij
Copy link

dswij commented Apr 27, 2024

I guess the question is whether reading after an ERROR_MORE_DATA gives you the second half of the datagram, or if anything beyond the end is just lost.

You can read the rest of the data by subsequent calls to ReadFile. It seems kind of expected to do so. See

What we can do is check for the total size of the message and adapt the buffer size before reading (by calling PeekNamedPipe).

@Thomasdezeeuw
Copy link
Collaborator Author

Closing in favour of #1778.

@Thomasdezeeuw Thomasdezeeuw deleted the win-handle-more-data branch May 9, 2024 17:34
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

Successfully merging this pull request may close these issues.

panic assert failed left: 4096 right: 0 - when sending > 4096 bytes in windows pipe message mode
4 participants