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

Certain ascii characters crash WASM when received from server fn #2377

Closed
jollygreenlaser opened this issue Feb 28, 2024 · 13 comments
Closed
Labels

Comments

@jollygreenlaser
Copy link
Contributor

jollygreenlaser commented Feb 28, 2024

EDIT: Current fix is to revert to nightly-2024-02-14: rustup default nightly-2024-02-14, may need to rustup target add wasm32-unknown-unknown again and if using cranelift, rustup component add rustc-codegen-cranelift-preview --toolchain nightly-2024-02-14. If using one, eg as done by default with cargo-leptos, update the rust-toolchain.toml file as well.


Describe the bug

Latest nightly breaks receiving certain characters from a server function, showing a dmalloc error. For me, it was en dash () and likely includes others. Unclear what caused this to suddenly break or what the true origin is.

This happens either upon navigating to the page with the server function call or during hydration when refreshing on that page.

This is very annoying for anything that has to display user entered data, eg profile bios.

Luckily the current workaround is pretty simple, just use this util all over the place:

pub fn clean_ascii(v: &String) -> String {
    v.chars()
        .filter(|c| *c <= '~')
        .collect::<String>()
}

This started happening after upgrading my nightly from late January. Due to testing a separate bug I've been able to repro this bug on 0.6.5, 0.6.6 and the tip of main version I'm on. I have a deployed 0.6.5, late January nightly version without this issue.

It feels very likely that the issue is in a dependency rather than Leptos proper. However, I've not been able to track this down at all despite a lot of effort and I'm guessing others will start to hit it too.

Leptos Dependencies

$ rustup -V
rustup 1.26.0 (5af9b9484 2023-04-05)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.78.0-nightly (fc3800f65 2024-02-26)`
[patch.crates-io]
leptos = { git = "https://github.com/leptos-rs/leptos.git", rev = "6d6019b956c81b2ec86e3aab7dd0e93db54dca3e" }
leptos_meta = { git = "https://github.com/leptos-rs/leptos.git", rev = "6d6019b956c81b2ec86e3aab7dd0e93db54dca3e" }
leptos_router = { git = "https://github.com/leptos-rs/leptos.git", rev = "6d6019b956c81b2ec86e3aab7dd0e93db54dca3e" }
leptos_axum = { git = "https://github.com/leptos-rs/leptos.git", rev = "6d6019b956c81b2ec86e3aab7dd0e93db54dca3e" }

https://github.com/leptos-rs/leptos/tree/6d6019b956c81b2ec86e3aab7dd0e93db54dca3e

Screenshot:

Screenshot 2024-02-27 at 10 21 12 PM

Text:

panicked at /rust/deps/dlmalloc-0.2.5/src/dlmalloc.rs:1182:13:
assertion failed: psize <= size + max_overhead

Stack:

Error
    at imports.wbg.__wbg_new_abda76e883ba8a5f (http://172.91.146.59:3000/pkg/local.js:530:13)
    at clepticle.wasm.console_error_panic_hook::hook::h8eb889a93dddf01a (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[3411]:0x103d005)
    at clepticle.wasm.core::ops::function::Fn::call::hdb33dbd3bd9e95b1 (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[15663]:0x130db10)
    at clepticle.wasm.std::panicking::rust_panic_with_hook::h8e9923a1fd637d6a (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[8913]:0x123fd15)
    at clepticle.wasm.std::panicking::begin_panic_handler::{{closure}}::h6dea320ce1182e58 (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[10394]:0x1285d4a)
    at clepticle.wasm.std::sys_common::backtrace::__rust_end_short_backtrace::ha27827269adf0111 (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[21114]:0x134bc43)
    at clepticle.wasm.rust_begin_unwind (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[16307]:0x1317b8b)
    at clepticle.wasm.core::panicking::panic_fmt::h97853b40b106d930 (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[17276]:0x1324e73)
    at clepticle.wasm.core::panicking::panic::h11ea08863e3a8cec (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[15642]:0x130d5b0)
    at clepticle.wasm.__rdl_dealloc (http://172.91.146.59:3000/pkg/local.wasm:wasm-function[13151]:0x12dd02e)
Uncaught (in promise) RuntimeError: unreachable
    at clepticle.wasm.__rust_start_panic (local.wasm:0x134c4c4)
    at clepticle.wasm.rust_panic (local.wasm:0x134abb1)
    at clepticle.wasm.std::panicking::rust_panic_with_hook::h8e9923a1fd637d6a (local.wasm:0x123fd42)
    at clepticle.wasm.std::panicking::begin_panic_handler::{{closure}}::h6dea320ce1182e58 (local.wasm:0x1285d4a)
    at clepticle.wasm.std::sys_common::backtrace::__rust_end_short_backtrace::ha27827269adf0111 (local.wasm:0x134bc43)
    at clepticle.wasm.rust_begin_unwind (local.wasm:0x1317b8b)
    at clepticle.wasm.core::panicking::panic_fmt::h97853b40b106d930 (local.wasm:0x1324e73)
    at clepticle.wasm.core::panicking::panic::h11ea08863e3a8cec (local.wasm:0x130d5b0)
    at clepticle.wasm.__rdl_dealloc (local.wasm:0x12dd02e)
    at clepticle.wasm.__rust_dealloc (local.wasm:0x134a953)
@benwis
Copy link
Contributor

benwis commented Feb 28, 2024

Stuff like this is why I switched to stable

@gbj
Copy link
Collaborator

gbj commented Feb 28, 2024

Do you have a minimal reproduction of a server function that can trigger this panic?

@jollygreenlaser
Copy link
Contributor Author

I'll have one soon.

Further investigation finds this works fine on nightly-2024-02-14 and fails on nightly-2024-02-15

The merged PR is rust-lang/rust#120777 which mentions doing unicode things

@OvermindDL1
Copy link
Contributor

OvermindDL1 commented Feb 28, 2024

For note, the dash you put in the post here is not ascii (it is indeed a UTF-8 en-dash in this case, but github 'unifies' everything to utf-8 in posts anyway regardless of its source), copy pasted into a rust repl:

>> "–".as_bytes()
[226, 128, 147]

Are you sure that in the page it's utf-8 and not something else like utf-16 or so?

@jollygreenlaser
Copy link
Contributor Author

Sorry, I'm bad with terminology on ascii vs utf. It is a character, that was given by a server function, that I used to be able to receive ok.

@OvermindDL1
Copy link
Contributor

If you debug in sending it on the server side pre-serialization is it parsed into a rust String and then sent over or is it kept as a javascript string or a byte array or something then sent? Can you make a minimal example case?

@jollygreenlaser
Copy link
Contributor Author

https://github.com/jollygreenlaser/ascii-death repro

In short:

#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct AsciiDeath {
    pub killer: String, // Also works with Vec<String>
    pub after: bool,    // Can be anything
}

#[server]
pub async fn kill() -> Result<AsciiDeath, ServerFnError> {
    Ok(AsciiDeath {
        // Needs something after it in the string
        // Seems to die on anything € or higher, aka not in character code 32-127
        killer: "€a".to_string(),
        // Needs something after it in the struct
        after: true,
    })
}

#[component]
fn HomePage() -> impl IntoView {
    let (count, set_count) = create_signal(0);
    let on_click = move |_| set_count.update(|count| *count += 1);

    // Can be create_resource or create_local_resource
    // With create_resource the page dies on hydration - the "click me" stops working
    // With create_local_resource it dies on fetch - the "click me" still works
    let deadly_data = create_resource(move || (), |_| async move { kill().await });
    
    // Not technically needed - will die even if this is an empty div
    view! {
        <h1>"Welcome to Leptos!"</h1>
        <button on:click=on_click>"Click Me: " {count}</button>
        <Suspense
            fallback=move || view! { <p>Loading...</p> }
        >
            {move || deadly_data.get().map(|res| match res {
                Ok(res) => {
                    log!("Saw res: {res:?}");
                    view! {
                        <p>{format!("Got data: {res:?}")}</p>
                    }
                },
                Err(err) => {
                    view! {
                        <p>{format!("Had error: {err:?}")}</p>
                    }
                },
            })}
        </Suspense>
    }
}

@OvermindDL1 thanks for the suggestion, will try that next

@jollygreenlaser
Copy link
Contributor Author

Also happens in Actix: https://github.com/jollygreenlaser/actix-death

@jollygreenlaser
Copy link
Contributor Author

Further testing shows that string length may have a minimum. What I thought was "requires something after in the struct" may actually be this.

It seems all that matters is number of bytes after the unwanted character.

Replacing kill with:

#[server]
pub async fn kill() -> Result<String, ServerFnError> {
    Ok("€aaaaaaaaaaaaaaa".to_string())
}

Also causes the same crash. Even just one fewer a doesn't crash. No amount of characters before will cause a crash on their own.

@purung
Copy link
Contributor

purung commented May 9, 2024

For anyone else running into the problem, this seems like a good issue thread.

Wasm-pack issue summary

@SFBdragon
Copy link

@jollygreenlaser How can I get ascii-death/actix-death running locally? (I've gotten as far as getting wasm-pack to build it successfully with my instrumented allocator.)

I'm hoping it might be possible to isolate the code producing the bad allocation with such a small repro.

@elpiel
Copy link

elpiel commented May 9, 2024

Hello,
Since we also had this issue and were trying to figure out where it's originating I wanted to link a few issues and raise attention to wasm-bindgen changes that have fixed this is in recent version.

There has been certain amount of investigation by @SFBdragon : rustwasm/wasm-pack#1389 (comment)

In summary, after 1.78 enabled debug assertions in dlmalloc, allocating and deallocating with a different size (this is UB) is caught instead of "silently working" (because of how dlmalloc works, specifically, other allocators will often crash as they actually rely on the safety contract).

Quoting my comment in the issue dlmalloc (alexcrichton/dlmalloc-rs#41):

For anyone landing on this issue, it seems that in the latest revision on main this issue has been fixed.
In the changelog on wasm-bindgen I see this bugfix related to UB in String deallocation, since we were using an older version of wasm-bindgen (because last time we tried to upgrade it was breaking our app again :D )

Fixed UB when freeing strings received from JS if not using the default allocator. rustwasm/wasm-bindgen#3808

https://github.com/rustwasm/wasm-bindgen/blob/0.2.92/CHANGELOG.md#fixed-1

OR

Take alignment into consideration during (de/re)allocation. rustwasm/wasm-bindgen#3463

https://github.com/rustwasm/wasm-bindgen/blob/0.2.92/CHANGELOG.md#0287

rustwasm/wasm-bindgen#3463

@jollygreenlaser
Copy link
Contributor Author

jollygreenlaser commented Aug 15, 2024

This seems to be working on latest nightly after upgrading to wasm-bindgen = "=0.2.93" (and other required leptos upgrades, namely 0.6.14, leptos-use to 0.12.0, reinstalling cargo-leptos, and of course a bit of cargo clean & deleting cargo.lock)

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

No branches or pull requests

7 participants