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

discord: use buildFHSUserEnv to avoid crashing #96246

Closed
wants to merge 1 commit into from

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Aug 25, 2020

Motivation for this change

got tired of it crashing

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jonringer jonringer requested review from benley, tadeokondrak and rileyinman and removed request for benley August 25, 2020 07:00
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/discord-0-0-11-package-sometimes-randomly-crashes/8725/6

@KamilaBorowska
Copy link
Member

KamilaBorowska commented Aug 25, 2020

This probably should be using buildFHSUserEnvBubblewrap (see #55973).

@JJJollyjim
Copy link
Member

JJJollyjim commented Aug 25, 2020

@jonringer are you sure this solves the root cause, and not a second crash? The error message is "Cannot upload crash dump: cannot exec /usr/bin/wget"!

The crash I am aware of is #96063, which would be resolved by #93453

@JJJollyjim
Copy link
Member

JJJollyjim commented Aug 25, 2020

(the only place in the binary wget is referenced is in code that looks like it corresponds to this part of CEF: https://chromium.googlesource.com/chromium/src/+/470748ce474f8fce7a566eb570021273c88d04c0/components/crash/app/breakpad_linux.cc#1086

@jonringer
Copy link
Contributor Author

@JJJollyjim yes, but allowing for the initial /usr/bin/wget to succeed makes the failure with libappindicator irrelevant.

@jonringer
Copy link
Contributor Author

probably should be using buildFHSUserEnvBubblewrap (see #55973).

done

@JJJollyjim
Copy link
Member

@jonringer sorry, I'm still not sure I understand. My conception of what's happening:

  • Discord is segfaulting in libappindicator, thus crashing
  • As part of crashing, it tries to phone home with wget, and fails

It's going to crash either way right?

@jonringer
Copy link
Contributor Author

jonringer commented Aug 25, 2020

@JJJollyjim you're absolutely right, please look at #96296

I was able to "hang" in a voice channel for 3 hours, previously I was crashing every <10mins

@jonringer jonringer closed this Aug 25, 2020
@jonringer jonringer deleted the fix-discord branch August 25, 2020 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants