Closed Bug 1060292 Opened 10 years ago Closed 10 years ago

PR_Assert() and PR_Abort() output is not visible by default on Android

Categories

(NSPR :: NSPR, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED FIXED
4.10.8

People

(Reporter: gsvelto, Assigned: gsvelto)

References

Details

Attachments

(1 file)

Both PR_Assert() and PR_Abort() invoke PR_LogPrint() unconditionally when they're called. PR_Assert() additionally explicitly writes to stderr so that its output is visible even if logging is entirely disabled (i.e. NSPR_LOG_MODULES is not set and thus logFile is null). Since both function interrupt execution it's important that their output is readily available otherwise the application will die without an apparent reason (see bug 881389 comment 41). We should explicitly write to the logcat directly on Android/FxOS in order for the calls' output to be always visible.
This patch makes PR_Abort() print unconditionally to the logcat when invoked and makes PR_Assert() similarly use __android_log_assert() to unconditionally output to the logcat when invoked. This mirrors the behavior on other platforms that unconditionally print to stderr and then invoke an assertion-specific function that traps before aborting so that a debugger has a chance to catch it before the program closes.
Assignee: wtc → gsvelto
Status: NEW → ASSIGNED
Attachment #8481195 - Flags: review?(wtc)
Comment on attachment 8481195 [details] [diff] [review] [PATCH] Make PR_Assert() and PR_Abort() output visible in the logcat on Android Review of attachment 8481195 [details] [diff] [review]: ----------------------------------------------------------------- r=wtc. Thanks for the patch. Patch checked in: https://hg.mozilla.org/projects/nspr/rev/acccc321337d ::: pr/src/io/prlog.c @@ +546,5 @@ > PR_IMPLEMENT(void) PR_Assert(const char *s, const char *file, PRIntn ln) > { > PR_LogPrint("Assertion failure: %s, at %s:%d\n", s, file, ln); > fprintf(stderr, "Assertion failure: %s, at %s:%d\n", s, file, ln); > fflush(stderr); Do you think we should skip the fprintf call on Android?
Attachment #8481195 - Flags: review?(wtc)
Attachment #8481195 - Flags: review+
Attachment #8481195 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 4.10.8
(In reply to Wan-Teh Chang from comment #2) > ::: pr/src/io/prlog.c > @@ +546,5 @@ > > PR_IMPLEMENT(void) PR_Assert(const char *s, const char *file, PRIntn ln) > > { > > PR_LogPrint("Assertion failure: %s, at %s:%d\n", s, file, ln); > > fprintf(stderr, "Assertion failure: %s, at %s:%d\n", s, file, ln); > > fflush(stderr); > > Do you think we should skip the fprintf call on Android? Normally, stderr is redirected to /dev/null on Android and B2G. It is possible, under B2G, to launch B2G from an adb shell and in that case, stderr will be visible. So I think that there is possibly some merit in keeping and I can't see any disadvantage in keeping it.
(In reply to Dave Hylands [:dhylands][on PTO - back Sep 11] from comment #3) > Normally, stderr is redirected to /dev/null on Android and B2G. > > It is possible, under B2G, to launch B2G from an adb shell and in that case, > stderr will be visible. So I think that there is possibly some merit in > keeping and I can't see any disadvantage in keeping it. +1, agreed.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: