Bug: RawFileStoreTest.testDownloadScript() fails on dev_4_4

[bpindelski]

Other two methods in the RawFileStoreTest class in OmeroJava run fine, whereas testDownloadScript() fails with

FAILED: testDownloadScript on null(integration.RawFileStoreTest)
omero.InternalException
    serverStackTrace = "java.lang.NullPointerException
                        	at omero.cmd.SessionI.unregisterServant(SessionI.java:626)
                        	at ome.services.blitz.fire.SessionManagerI.onApplicationEvent(SessionManagerI.java:294)
                        	at org.springframework.context.event.SimpleApplicationEventMulticaster.multicastEvent(SimpleApplicationEventMulticaster.java:97)
                        	at ome.services.messages.GlobalMulticaster.multicastEvent(GlobalMulticaster.java:55)
                        	at ome.system.OmeroContext.publishEvent(OmeroContext.java:327)
                        	at ome.system.OmeroContext.publishMessage(OmeroContext.java:341)
                        	at ome.services.blitz.impl.AbstractCloseableAmdServant.close_async(AbstractCloseableAmdServant.java:103)
                        	at omero.api._RawFileStoreTie.close_async(_RawFileStoreTie.java:120)
                        	at omero.api._StatefulServiceInterfaceDisp.___close(_StatefulServiceInterfaceDisp.java:142)
                        	at omero.api._RawFileStoreDisp.__dispatch(_RawFileStoreDisp.java:327)
                        	at IceInternal.Incoming.invoke(Incoming.java:159)
                        	at Ice.ConnectionI.invokeAll(ConnectionI.java:2037)
                        	at Ice.ConnectionI.message(ConnectionI.java:972)
                        	at IceInternal.ThreadPool.run(ThreadPool.java:577)
                        	at IceInternal.ThreadPool.access$100(ThreadPool.java:12)
                        	at IceInternal.ThreadPool$EventHandlerThread.run(ThreadPool.java:971)
                        "
    serverExceptionClass = "java.lang.NullPointerException"
    message = ""
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
	at java.lang.Class.newInstance0(Class.java:357)
	at java.lang.Class.newInstance(Class.java:310)
	at IceInternal.BasicStream$DynamicUserExceptionFactory.createAndThrow(BasicStream.java:2243)
	at IceInternal.BasicStream.throwException(BasicStream.java:1632)
	at IceInternal.Outgoing.throwUserException(Outgoing.java:442)
	at omero.api._RawFileStoreDelM.close(_RawFileStoreDelM.java:408)
	at omero.api.RawFileStorePrxHelper.close(RawFileStorePrxHelper.java:595)
	at omero.api.RawFileStorePrxHelper.close(RawFileStorePrxHelper.java:567)
	at integration.RawFileStoreTest.testDownloadScript(RawFileStoreTest.java:117)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:84)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:714)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:901)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1231)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:127)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:111)
	at org.testng.TestRunner.privateRun(TestRunner.java:767)
	at org.testng.TestRunner.run(TestRunner.java:617)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:335)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:330)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:291)
	at org.testng.SuiteRunner.run(SuiteRunner.java:240)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1224)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1149)
	at org.testng.TestNG.run(TestNG.java:1057)
	at org.testng.remote.RemoteTestNG.run(RemoteTestNG.java:111)
	at org.testng.remote.RemoteTestNG.initAndRun(RemoteTestNG.java:204)
	at org.testng.remote.RemoteTestNG.main(RemoteTestNG.java:175)

[mtbcarroll]

When we hit AbstractCloseableAmdServant.close_async both ctx and holder are null.

[jamoore]

This is another case of the holder not being set on a servant. There has been a rash of these. If you'd prefer me to take a look, please pass it to me. Otherwise, these commits show how this has been settled elsewhere:

$ git log -S setHolder components/blitz/
commit cd67b37825cafef41487ae2a85a10adbe90f3095
Author: jmoore <josh@glencoesoftware.com>
Date:   Mon Dec 3 12:14:07 2012

    Use sf.configureServant in favor of msg.setHolder

    The addition of the holder field to RegisterServantMessage
    was a leaky abstraction. Rather than requiring that API
    usage, having the newly refactored method `configureServant`
    called on the servant during message processing is much
    cleaner and guarantees that the hard-wired AOP is applied.

commit 677add5d0bb900cbb378946d1f6545ee4cd31610
Author: jmoore <josh@glencoesoftware.com>
Date:   Wed Oct 10 20:01:57 2012

    More fixes for the servant test (WIP)

    These are still failing due to missing
    translation between calls on the proxies
    and the relevant servants. It might be
    easier to use an Ice.Communicator for the
    interaction.

commit 73a2febeff5c03ec747e039d7bb4c123d81c9d24
Author: jmoore <josh@glencoesoftware.com>
Date:   Tue Oct 2 11:28:25 2012

    Another NPE fix for RepoRawFileStoreI (See #6349)

    The holder must also be set in order to prevent an NPE
    during cleanup. I went ahead and took the time to refactor
    the two uses into a single method. This metho could possibly
    be moved into the ctor of RepoRFS itself.

commit 5a51901a25106e372947fb6eb63eddea92b306d9
Author: jmoore <josh@glencoesoftware.com>
Date:   Fri Aug 10 16:56:39 2012

    Fix compile error in SessionI

    SessionI cannot have references to ome.services.blitz.*
    classes. Instead, the value set needs to happen in

        ServiceFactoryI.internalServantConfig

commit 36004a8eb71ef4f915126a3348c20da0d6a88197
Author: jmoore <josh@glencoesoftware.com>
Date:   Fri Aug 10 15:43:58 2012

    Pass ServantHolder to AbstractAmdServants (See #9456)

    Without a valid clientId, it wasn't possible to find the
    ServiceFactoryI instance which owned the servant on close.

[mtbcarroll]

fixed by ​https://github.com/openmicroscopy/openmicroscopy/pull/1429

[Mark Carroll <m.t.b.carroll@…>]

(In [8b804b4778c9ceb7a695b920d1fcbdc21cd53ef5/ome.git]on branches master, dev_4_4) fix #11360: use message to pass servant holder into file store

[Josh Moore <josh@…>]

(In [e8c36bb834f516f87e366c1866e56919879a3f2e/ome.git]on branches master, dev_4_4) Merge pull request #1429 from mtbc/servant-holder-11360

fix #11360: get the servant holder from the session factory into the raw file store