Seattle on Android crash...

[choksi81]

An android user reports (via crash report):

java.lang.NullPointerException
at com.seattletestbed.process.InterpreterProcess.getHost(InterpreterProcess.java:96)
at com.seattletestbed.process.InterpreterProcess.<init>(InterpreterProcess.java:77)
at com.seattletestbed.process.PythonScriptProcess.<init>(PythonScriptProcess.java:35)
at com.seattletestbed.process.SeattleScriptProcess.<init>(SeattleScriptProcess.java:54)
at com.seattletestbed.process.SeattleScriptProcess.launchScript(SeattleScriptProcess.java:30)
at com.seattletestbed.ScriptService.startSeattleMain(ScriptService.java:238)
at com.seattletestbed.ScriptService.access$7(ScriptService.java:191)
at com.seattletestbed.ScriptService$2.run(ScriptService.java:246)
at com.seattletestbed.process.Process$1.run(Process.java:155)
at java.lang.Thread.run(Thread.java:1019)

We need to dig into this.

[choksi81]

Hmmm,
a problem during shutdown (Process.java:155, no indication whether it was user-initiated or due to a bug),
during which the automatic restart functionality triggered (logic above ScriptService.java:246),
in startSeattleMain (ScriptService.java:191),
started after we fell through the logic checking whether the main process already runs (logic above ScriptService.java:238) by
launching a new script process (SeattleScriptProcess.java:30),
which requires SeattleScriptProcess's parent class PythonScriptProcess to be initialized (SeattleScriptProcess.java:54),
which itself initialized its parent class InterpreterProcess (PythonScriptProcess.java:35),
which in the process of setting up environment variables for our nodemanager called getHost() (InterpreterProcess.java:77),
which tried to use the mProxy object that was null at this time (InterpreterProcess.java:96).
You can track back mProxy, it should be instantiated in ScriptService.java:234. We should figure out a way to handle nulls returned from the AndroidProxy constructor, depending on understanding first why exactly it will ever return null of course!

[choksi81]

You can track back mProxy, it should be instantiated in ScriptService.java:234.
... and is handed upwards from there.

[choksi81]

The problem is due to a race condition involving the initialization of mProxy. The main thread starts a new thread to perform this initialization. The main thread relies on the value of this.mProxy to be set before it proceeds with initialization. As such, when mProxy is created, it is assigned directly to this.mProxy. Therefore, there is no guarantee that this.startLocal() has finished executing by that point.
Another traceback I found on the Developer console I suspect to be related:
java.lang.NullPointerException
at com.googlecode.android_scripting.SimpleServer.shutdown(SimpleServer.java:230)
at com.googlecode.android_scripting.jsonrpc.JsonRpcServer.shutdown(JsonRpcServer.java:58)
at com.googlecode.android_scripting.AndroidProxy.shutdown(AndroidProxy.java:81)
at com.sensibilitytestbed.ScriptService$2.run(ScriptService.java:322)
at com.sensibilitytestbed.process.Process$1.run(Process.java:156)
at java.lang.Thread.run(Thread.java:841)
But I suspect that my patch will also address that as well. Albert will be building a new APK with my patch as I am failing to build it properly: https://seattle.poly.edu/attachment/ticket/1199/nullpointer.patch

[choksi81]

Indeed, I overlooked in my interpretation that the null pointer reference in !InterpreterProcess could have been a result of mAddress in [ AndroidProxy?.getHost()] not being initialized because startLocal wasn't called (or finished) yet, rather than a missing mProxy.
Leonard's patch instantiates a temporary AndroidProxy, .startLocal()'s it, and ensures that mAddress exists before mProxy is assigned the temp proxy's reference and the running proxy is made available.
I'll apply it now and rebuild.

[choksi81]

There's a small goof in the patch for InstallerService.java. This

•           AndroidProxy androidProxy = new AndroidProxy(s, null, true);
  

•           InetSocketAddress proxyAddress = null;
  

•           while (proxyAddress == null) {
  

•               Log.i(Common.LOG_TAG, "Starting mProxy... (InstallerService)");
  

•               proxyAddress = mProxy.startLocal();
  

•           }
  

•           mProxy = androidProxy;
  

  should obviously not use the uninitialized mProxy to try startLocal(). Corrected:

•           AndroidProxy androidProxy = new AndroidProxy(s, null, true);
  

•           InetSocketAddress proxyAddress = null;
  

•           while (proxyAddress == null) {
  

•               Log.i(Common.LOG_TAG, "Starting mProxy... (InstallerService)");
  

•               proxyAddress = androidProxy.startLocal();
  

•           }
  

•           mProxy = androidProxy;
  

  The correctly patched Sensibility binary works fine!

[choksi81]

Committed for Sensibility Testbed in r7214. The same fix needs to be applied to SeattleOnAndroid too!

[choksi81]

Attachments:
https://github.com/SeattleTestbed/attic/blob/master/TICKET_ATTACHMENTS/nullpointer.patch