781735 - SUTAgent should not die if it can't create a file on the sdcard

Status: RESOLVED FIXED

(Testing Graveyard :: SUTAgent, defect)

Description

[Justin Wood (:Callek)]

Attachment: v1

So while trying to guess/figure out why SUTAgent1.11 seems[ed] to be hanging/dying on its SUTAgent Command Port connections since upgrading...

I theorized that there are times (such as after a reboot) where it is launched/takes a command before the SDCard is fully ready (as in writable/available)

And it turns out that in some cases if |PrintWriter| fails to construct (e.g. is created "null" then we attempt to dereference null and call |close()|.

Of course this is a bad idea, and this patch guards against that, basically ignoring the fact that this happens, and allowing the command to do its normal run and do its other stuff.

This is all related to Bug 778784 and Bug 776078. And since I don't have a Java Build Environ, nor a local tegra/device to test with, this patch is untested

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 650790 [details] [diff] [review]
v1

Review of attachment 650790 [details] [diff] [review]:
-----------------------------------------------------------------

good find, I want to scour the code in a bit more detail to look for other culprits, but this might be all there is to it.

Comment 2

[William Lachance (:wlach)]

(In reply to Joel Maher (:jmaher) from comment #1)
> Comment on attachment 650790 [details] [diff] [review]
> v1
> 
> Review of attachment 650790 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> good find, I want to scour the code in a bit more detail to look for other
> culprits, but this might be all there is to it.

Looks ok to me too. 

We do have a bunch of other code in the agent that does stuff with files (and thus may fail with a bad sdcard), but I think the fairly unique thing we have here is the finally clause that gets executed whether or not we successfully open/write to the file.

I'm not actually sure if the finally clause is actually necessary at all (won't it be called explicitly by the destructor regardless?) but I couldn't find anything saying that for sure with a quick google search. I say we apply this as-is.

Comment 3

[Justin Wood (:Callek)]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/9676734d2960
Pushed with don't build

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9676734d2960

Comment 5

[Ed Morley [:emorley]]

(In reply to Justin Wood (:Callek) from comment #3)
> Pushed with don't build

<3 :-)