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

Regression in v0.21.0 panic occurred at minidump-stackwalk/src/main.rs:430 #967

Closed
relud opened this issue Feb 28, 2024 · 3 comments · Fixed by #968
Closed

Regression in v0.21.0 panic occurred at minidump-stackwalk/src/main.rs:430 #967

relud opened this issue Feb 28, 2024 · 3 comments · Fixed by #968
Assignees

Comments

@relud
Copy link
Contributor

relud commented Feb 28, 2024

found when regression testing in mozilla-services/socorro-stackwalk#13

dockerfile:

FROM rust:1.76.0-bullseye@sha256:b3ec72b36c32f9c2437714354fadf2d05988acd3333699145e0a539c524bde99

ARG groupid=10001
ARG userid=10001

WORKDIR /app/

RUN apt update && apt install -y python3-venv

RUN update-ca-certificates && \
    groupadd --gid $groupid app && \
    useradd -g app --uid $userid --shell /usr/sbin/nologin --create-home app && \
    chown app:app /app/

USER app

setup:

$ docker build . -t local/minidump-stackwalk
$ docker run --rm -ti local/minidump-stackwalk
$ cargo install --locked --target=x86_64-unknown-linux-gnu --root=./build/ --git https://github.com/rust-minidump/rust-minidump.git --rev v0.21.0 --profile release --force minidump-stackwalk
$ python3 -m venv venv
$ . venv/bin/activate
$ pip install crashstats_tools
$ export CRASHSTATS_API_TOKEN=redacted
$ fetch-data --raw --dumps regr/crashdata af300837-1e4e-44b1-b8d0-e0a6b0240228
Using api token: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Working on af300837-1e4e-44b1-b8d0-e0a6b0240228...
Fetching raw af300837-1e4e-44b1-b8d0-e0a6b0240228
Fetching dump af300837-1e4e-44b1-b8d0-e0a6b0240228/upload_file_minidump
Done!
$ mkdir -p symbols_cache/{cache,tmp} regr/20240228_234147/nocache/

failure on v0.21.0:

$ build/bin/minidump-stackwalk --evil-json=regr/crashdata/raw_crash/20240228/af300837-1e4e-44b1-b8d0-e0a6b0240228 --symbols-cache=symbols_cache/cache --symbols-tmp=symbols_cache/tmp --no-color --symbols-url=https://symbols.mozilla.org --output-file=regr/20240228_234147/nocache/output.af300837-1e4e-44b1-b8d0-e0a6b0240228.json --json --verbose=error regr/crashdata/upload_file_minidump/af300837-1e4e-44b1-b8d0-e0a6b0240228
ERROR Panic - A panic occurred at minidump-stackwalk/src/main.rs:430: need system info: StreamNotFound

behavior on v0.20.0:

$ cargo install --locked --target=x86_64-unknown-linux-gnu --root=./build/ --git https://github.com/rust-minidump/rust-minidump.git --rev v0.20.0 --profile release --force minidump-stackwalk
$ build/bin/minidump-stackwalk --evil-json=regr/crashdata/raw_crash/20240228/af300837-1e4e-44b1-b8d0-e0a6b0240228 --symbols-cache=symbols_cache/cache --symbols-tmp=symbols_cache/tmp --no-color --symbols-url=https://symbols.mozilla.org --output-file=regr/20240228_234147/nocache/output.af300837-1e4e-44b1-b8d0-e0a6b0240228.json --json --verbose=error regr/crashdata/upload_file_minidump/af300837-1e4e-44b1-b8d0-e0a6b0240228
ERROR MissingThreadList - Error processing dump: The thread list stream was not found
@gabrielesvelto
Copy link
Collaborator

Thanks Daniel! We introduced this regression in b5c57e8085218716f1cedcfe4f46719ddd27293a. We failed cleanly before, raising a MissingThreadList error because the minidump is (mostly) empty, we should reintroduce the old behavior instead of panicking. @afranchuk can you take care of this and I'll cut a dot release with the fix?

@afranchuk afranchuk self-assigned this Feb 29, 2024
afranchuk added a commit to afranchuk/rust-minidump that referenced this issue Feb 29, 2024
Otherwise it is not required and the program shouldn't panic.

Closes rust-minidump#967.
afranchuk added a commit to afranchuk/rust-minidump that referenced this issue Feb 29, 2024
Otherwise it is not required. In either case it shouldn't panic.

Closes rust-minidump#967.
@afranchuk
Copy link
Collaborator

@gabrielesvelto ready for that patch release, unless you think the change I made isn't enough (I tested to confirm it fixes the regression).

@gabrielesvelto
Copy link
Collaborator

This is fine, I'll cut another release

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 a pull request may close this issue.

3 participants