Closed
Bug 894454
Opened 11 years ago
Closed 11 years ago
sutagent crashes during testroot if /data/local/tmp cannot be accessed
Categories
(Testing :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla25
People
(Reporter: gbrown, Assigned: gbrown)
Details
Attachments
(1 file)
3.72 KB,
patch
|
wlach
:
review+
|
Details | Diff | Splinter Review |
If the sutagent cannot access /data/local/tmp while executing testroot, an NPE occurs and the sutagent crashes: I/SUTAgentAndroid( 1939): 10.0.2.2 : testroot I/SUTAgentAndroid( 1939): Caught exception creating file in /data/local/tmp: open failed: EACCES (Permission denied) E/SUTAgentAndroid( 1939): ERROR: Cannot access world writeable test root W/dalvikvm( 1939): threadid=13: thread exiting with uncaught exception (group=0xb2e1a908) E/AndroidRuntime( 1939): FATAL EXCEPTION: CmdWorkerThread E/AndroidRuntime( 1939): java.lang.NullPointerException E/AndroidRuntime( 1939): at com.mozilla.SUTAgentAndroid.service.CmdWorkerThread.run(CmdWorkerThread.java:142) W/ActivityManager( 1187): Force finishing activity com.mozilla.SUTAgentAndroid/.SUTAgentAndroid This happened on an emulator running sutagent 1.18. It looks to me like this behavior existed previous to 1.18. I think it would be better to return an error message from testroot and continue running.
Assignee | ||
Comment 1•11 years ago
|
||
BTW, this failure happened because 'su' was not correctly installed on the emulator: without full su powers, sutagent could not change the permissions on /data/local/tmp.
Assignee | ||
Comment 2•11 years ago
|
||
I don't know of any way to bring about the NPE crash other than the specific, invalid configuration that I reported, but just in case... - return an error message from GetTestRoot if test root cannot be determined - avoid NPE if any command handler returns null
Attachment #785147 -
Flags: review?(wlachance)
Comment 3•11 years ago
|
||
Comment on attachment 785147 [details] [diff] [review] avoid NullPointerExceptions in sutagent Review of attachment 785147 [details] [diff] [review]: ----------------------------------------------------------------- I think some of this isn't necessary. r+ with unnecessary changes removed. If you want to keep that stuff, let's discuss. :) ::: build/mobile/sutagent/android/CmdWorkerThread.java @@ +146,2 @@ > if (outputLine.length() > 0) > { I don't think this change is necessary. We really shouldn't be returning null for processCommand, afaict. Inspecting DoCommand, I can't see other cases where we were other than the one that you fixed. ::: build/mobile/sutagent/android/DataWorkerThread.java @@ +172,5 @@ > outputLine = dc.processCommand(inputLine, out, in, cmdOut); > + if (outputLine == null) > + { > + outputLine = ""; > + } As above?
Attachment #785147 -
Flags: review?(wlachance) → review+
Assignee | ||
Comment 4•11 years ago
|
||
I want to keep the "unnecessary changes". There are a lot of code paths returning from processCommand - more than I can inspect reliably! - and some of them return the value returned from Android APIs. Also, we add new commands from time to time. If we miss a null return, we hit an NPE on outputLine.length(), sutagent goes down, a test reports something cryptic like "timeout in command...", the machine gets rebooted, and we have almost no way to diagnose the problem -- that's worth preventing.
Comment 5•11 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #4) > I want to keep the "unnecessary changes". There are a lot of code paths > returning from processCommand - more than I can inspect reliably! - and some > of them return the value returned from Android APIs. Also, we add new > commands from time to time. > > If we miss a null return, we hit an NPE on outputLine.length(), sutagent > goes down, a test reports something cryptic like "timeout in command...", > the machine gets rebooted, and we have almost no way to diagnose the problem > -- that's worth preventing. Ok, go ahead and commit as-is. :) This really feels wrong, but I guess there are problems further up the chain that lead us to need to paper over problems like this.
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/19f48fd7faf1
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/19f48fd7faf1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•