Massive allocation on dumps::from_binary

[emi2k01]

When using dumps::from_binary, the program aborts with the following message:

memory allocation of 7881122326526296064 bytes failed

This doesn't happen in v4.6. Only in v4.7.

The dump read is exported with dump_to_file.

To reproduce:

fn main() {
    let dump_path: &'static [u8] = include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/syntaxset.packdump"));
    let _: syntect::parsing::SyntaxSet = syntect::dumps::from_binary(dump_path);
}

And put a dump in the same directory as Cargo.toml with the name syntaxset.packdump. I used zola's

Only happens on computers with less than 7881 petabytes of ram.

[Enselic]

If the dump was created with 4.7 it should load with 4.7. A dump created with 4.6 or earlier will not load in 4.7.

the gigantic alloc is because the binary format changed due to lazy loading, so it essentially allocs a random amount of memory.

This means a major bump is needed and 4.7.1 must be yanked, but none of the two owners of the crate on crates.io has had time to do so yet.

[Enselic]

See #403 (comment)

[trishume]

Aww crap I tend to save my syntect emails for weekends and only see the subject lines until then so didn't notice the need for a yank until now. I just yanked v4.7.1

[Enselic]

Thanks Tristan, sorry again to mess this up, and good to know how you organize your work.

Next time (which hopefully never happens) I'll make sure to not reuse existing issues and PRs when commenting, so that you are alerted from the title of a new issue 👍

[emi2k01]

Thank you both!

I'm not sure if this issue should be closed now that the version was yanked or until a fix is released. I'll leave it up to you.

[trishume]

I'll close it. We won't actually fix anything, just release the new code as a new major version.

[Canop]

Problem with this issue being closed is it's hard to find. I bissected up to the faulty commit before finding the issue. I have no suggestion, just pointing the problem.

[Enselic]

Sorry to hear this stills causes problems.

Did you go by git tag? Do you think you would have noticed if we added a disclaimer to the git tag descriptions?

If you used normal crates.io dependencies, how come cargo did not warn that the latest releases were yanked?

Another option would be to delete the tags. I am skeptical about that however, since the versions exist on crates.io.

If you don't think "anything" would have saved you this trouble except reopening this issue, then I will do that.

[Canop]

@Enselic I did try to use master instead of a crates.io version because the 4.6 version has a panic (issue #421) and I wanted to see whether the problem was still here and whether I could fix it. As the memory allocation failure doesn't come with a location, it didn't occur to me that the problem was an incompatibility with the dump file.
Perhaps this issue should be kept open until the 5.0 solves it ?
Or perhaps I should have just looked at closed issues before everything else.

[Enselic]

Ok I see. I agree it makes sense to keep this open for a while longer for increased visibility. I'll reopen it.

[Enselic]

@Canop and others: 5.0.0 has been out for some days now, so maybe we can close this issue now?