It should be relatively obvious what's wrong here: - if (now.tv_sec > timeLimit.tv_nsec || - (now.tv_sec == timeLimit.tv_nsec && + if (now.tv_sec > timeLimit.tv_sec || + (now.tv_sec == timeLimit.tv_sec && now.tv_nsec > timeLimit.tv_nsec)) { This code matters only if a thread has masked the signal we're using to try to tell it to enable seccomp-bpf (or otherwise takes longer than 10 seconds to answer), so this wasn't noticed until now. The time is CLOCK_MONOTONIC, which seems to be approximately the time since boot, so reaching a significant fraction of 1000000000 seconds (~31 years) in order to exceed the (effectively random) nanosecond count is unlikely. Thus we very likely hang forever instead of crashing, which kind of defeats the point of having this timeout.

Comment on attachment 8624557 [details] [diff] [review] Patch missed it :(

(In reply to Guillaume Destuynder [:kang] from comment #2) > missed it :( At least two other people also missed it, for what it's worth. Ideally it would have been caught by testing, but there wasn't any reasonable way to write a test case for it.

Skipping Try because: this is extremely unlikely to break the build, and testing doesn't cover this error case. About the tracking flags: the bug theoretically affects everything since B2G 1.3 and Firefox 33, but since the error case isn't known to have ever happened on a normal build, I don't plan on requesting uplift.

Bug 1187408 looks like it's a few reported cases of this bug showing up in practice — as a false positive, effectively making the timeout 10ms instead of 10s with low but non-negligible probability. The patch is very small and self-contained, so uplift shouldn't be risky.

(In reply to Jed Davis [:jld] {UTC-7} from comment #8) > Bug 1187408 looks like it's a few reported cases of this bug showing up in > practice — as a false positive, effectively making the timeout 10ms instead > of 10s with low but non-negligible probability. The patch is very small and > self-contained, so uplift shouldn't be risky. Hi Jed, Please raise b2g37 uplift request. Thanks.

Slight problem: this seems to be the regressing changeset for bug 1185118. How much of a problem this is depends on what's actually going on there — if the process was going to hang indefinitely due to a kernel bug, then crashing it after 10 seconds is arguably an improvement, but if it's just extremely slow for some reason then that's probably bad. On the other hand, the processes that would be incorrectly killed by this bug (as seen in bug 1187408) are probably ones that would have behaved correctly otherwise.

Bug 1187408 has been closed by the submitter('s test management bot), so I'm clearing the 2.2+ because this bug no longer blocks a 2.2 blocker.