Make the signal handler exit immediately #22259

jdm · 2018-11-23T21:28:26Z

This addresses #22240 by immediately exiting, rather than sometimes triggering a SIGBUS by aborting that can be caught recursively.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix Crash handler invoking intrinsics::abort sometimes causes recursive crash handling to occur #22240
There are tests for these changes

This change is

highfive · 2018-11-23T21:28:29Z

Heads up! This PR modifies the following files:

@paulrouget: ports/servo/non_android_main.rs, ports/servo/Cargo.toml

Manishearth · 2018-11-24T20:19:16Z

@bors-servo r+

bors-servo · 2018-11-24T20:19:16Z

📌 Commit 84ae607 has been approved by Manishearth

bors-servo · 2018-11-24T20:19:23Z

⌛ Testing commit 84ae607 with merge 72e9753...

Make the signal handler exit immediately This addresses #22240 by immediately exiting, rather than sometimes triggering a SIGBUS by aborting that can be caught recursively. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #22240 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22259)

bors-servo · 2018-11-24T20:21:05Z

💔 Test failed - windows-msvc-dev

CYBAI · 2018-11-26T12:31:54Z

error[E0432]: unresolved imports `libc::pid_t`, `libc::kill`
 --> C:\buildbot\.cargo\registry\src\github.com-1ecc6299db9ec823\sig-1.0.0\src\ffi.rs:8:16
  |
8 | pub use libc::{pid_t, c_int, getpid, kill};
  |                ^^^^^                 ^^^^ no `kill` in the root
  |                |
  |                no `pid_t` in the root

error[E0432]: unresolved imports `libc::sigaction`, `libc::sighandler_t`, `libc::sigfillset`
  --> C:\buildbot\.cargo\registry\src\github.com-1ecc6299db9ec823\sig-1.0.0\src\lib.rs:27:12
   |
27 | use libc::{sigaction, sighandler_t, sigfillset};
   |            ^^^^^^^^^  ^^^^^^^^^^^^  ^^^^^^^^^^ no `sigfillset` in the root
   |            |          |
   |            |          no `sighandler_t` in the root
   |            no `sigaction` in the root

error: aborting due to 2 previous errors

jdm · 2018-11-26T15:49:36Z

@bors-servo r=Manishearth

bors-servo · 2018-11-26T15:49:37Z

📌 Commit 89ee79a has been approved by Manishearth

bors-servo · 2018-11-26T17:44:37Z

⌛ Testing commit 89ee79a with merge eb44615...

Make the signal handler exit immediately This addresses #22240 by immediately exiting, rather than sometimes triggering a SIGBUS by aborting that can be caught recursively. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #22240 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22259)

bors-servo · 2018-11-26T17:49:06Z

💔 Test failed - linux-rel-wpt

CYBAI · 2018-11-27T01:52:47Z

In linux-rel-wpt:

error: unused `#[macro_use]` import
 --> ports/servo/non_android_main.rs:6:30
  |
6 | #[cfg(feature = "unstable")] #[macro_use] extern crate sig;
  |                              ^^^^^^^^^^^^
  |
  = note: `-D unused-imports` implied by `-D warnings`

error: unused import: `libc::_exit`
  --> ports/servo/non_android_main.rs:16:5
   |
16 | use libc::_exit;
   |     ^^^^^^^^^^^

error: aborting due to 2 previous errors

In windows-msvc-dev:

error[E0463]: can't find crate for `sig`
 --> ports\servo\non_android_main.rs:6:43
  |
6 | #[cfg(feature = "unstable")] #[macro_use] extern crate sig;
  |                                           ^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

jdm · 2018-11-27T03:52:03Z

@bors-servo r=Manishearth

jdm · 2019-02-01T15:26:05Z

@bors-servo r=Manishearth

bors-servo · 2019-02-01T15:26:06Z

📌 Commit 708031c has been approved by Manishearth

bors-servo · 2019-02-01T15:26:11Z

⌛ Testing commit 708031c with merge cd5c0dc...

Make the signal handler exit immediately This addresses #22240 by immediately exiting, rather than sometimes triggering a SIGBUS by aborting that can be caught recursively. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #22240 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22259)

bors-servo · 2019-02-01T15:48:22Z

💔 Test failed - linux-rel-css

CYBAI · 2019-02-01T15:55:44Z

💔 Test failed - linux-rel-css

Failed at shell__5 'bash ./etc/ci/lockfile_changed.sh' failed

diff --git a/Cargo.lock b/Cargo.lock
index 600c3cd..58f8b71 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3640,7 +3640,7 @@ dependencies = [
  "glutin 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "keyboard-types 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)",
  "libservo 0.0.1",
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "osmesa-src 0.1.0 (git+https://github.com/servo/osmesa-src)",
@@ -3924,7 +3924,7 @@ name = "sig"
 version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

jdm · 2019-02-01T17:17:17Z

@bors-servo r+
The dangers of rebasing old code.

bors-servo · 2019-02-01T17:17:18Z

📌 Commit 342b9ef has been approved by jdm

bors-servo · 2019-02-01T17:17:23Z

⌛ Testing commit 342b9ef with merge 4a6b911...

Make the signal handler exit immediately This addresses #22240 by immediately exiting, rather than sometimes triggering a SIGBUS by aborting that can be caught recursively. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #22240 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22259)

bors-servo · 2019-02-01T18:34:30Z

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: jdm
Pushing 4a6b911 to master...

highfive assigned Manishearth Nov 23, 2018

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 23, 2018

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 24, 2018

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 24, 2018

jdm force-pushed the signal branch from 84ae607 to 89ee79a Compare November 26, 2018 15:49

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 26, 2018

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 26, 2018

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 26, 2018

jdm force-pushed the signal branch from 89ee79a to 760cd13 Compare November 27, 2018 03:49

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 27, 2018

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 1, 2019

Exit the process from the signal handler.

1785009

jdm force-pushed the signal branch from 3d8c1d1 to 708031c Compare February 1, 2019 15:25

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Feb 1, 2019

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Feb 1, 2019

Upgrade sig dependency.

342b9ef

jdm force-pushed the signal branch from 708031c to 342b9ef Compare February 1, 2019 17:16

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Feb 1, 2019

highfive assigned jdm and unassigned Manishearth Feb 1, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Feb 1, 2019

bors-servo merged commit 342b9ef into servo:master Feb 1, 2019

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 1, 2019

atouchet mentioned this pull request Feb 1, 2019

Audit our crash handler #12162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the signal handler exit immediately #22259

Make the signal handler exit immediately #22259

jdm commented Nov 23, 2018 •

edited by SimonSapin

Loading

highfive commented Nov 23, 2018

Manishearth commented Nov 24, 2018

bors-servo commented Nov 24, 2018

bors-servo commented Nov 24, 2018

bors-servo commented Nov 24, 2018

CYBAI commented Nov 26, 2018

jdm commented Nov 26, 2018

bors-servo commented Nov 26, 2018

bors-servo commented Nov 26, 2018

bors-servo commented Nov 26, 2018

CYBAI commented Nov 27, 2018

jdm commented Nov 27, 2018

jdm commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

CYBAI commented Feb 1, 2019

jdm commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

Make the signal handler exit immediately #22259

Make the signal handler exit immediately #22259

Conversation

jdm commented Nov 23, 2018 • edited by SimonSapin Loading

highfive commented Nov 23, 2018

Manishearth commented Nov 24, 2018

bors-servo commented Nov 24, 2018

bors-servo commented Nov 24, 2018

bors-servo commented Nov 24, 2018

CYBAI commented Nov 26, 2018

jdm commented Nov 26, 2018

bors-servo commented Nov 26, 2018

bors-servo commented Nov 26, 2018

bors-servo commented Nov 26, 2018

CYBAI commented Nov 27, 2018

jdm commented Nov 27, 2018

jdm commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

CYBAI commented Feb 1, 2019

jdm commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

bors-servo commented Feb 1, 2019

jdm commented Nov 23, 2018 •

edited by SimonSapin

Loading