Re: [udig-devel] IService display thread safe

[Jody Garnett <jody.garnett@xxxxxxxxx>]

TLDR: Fixed it, pull request shortly

Okay cool; I tracked this down to two mistakes and will raise a pull request shortly.

a) IService.resolve was calling createInfo each time (rather then use getInfo which caches)

b) The quick label provider used from the display thread was calling getInfo(), this is why we made the getTitle and getIcon method (which could check the cached info if available), the slow label decorator will correctly call getInfo from a background thread later it it was not already available.

-- 

Jody Garnett

On Thursday, 12 July 2012 at 11:47 AM, Jody Garnett wrote:

I have been having bit of fun with a custom IService implementation that contains the following check:

        if (Display.getCurrent() != null) {

            throw new IllegalStateException(

                    "Access to the server not available from the display thread"); //$NON-NLS-1$

        }

I have found this *very* helpful for catching code that has a chance of blocking the display thread while it waits for IO. 

However it has turned up a few instances in uDig where we are incorrectly asking for IServiceInfo from the display thread under the assumption someone already loaded it. After our next release I would like to put a few Display thread checks (such as the above) in order to catch this kind of mistake early.

For reference where I get the problem in uDig is here:

at net.refractions.udig.catalog.internal.ServiceFactoryImpl.createService(ServiceFactoryImpl.java:241)

at net.refractions.udig.catalog.ServiceParameterPersister.locateService(ServiceParameterPersister.java:241)

at net.refractions.udig.catalog.ServiceParameterPersister.restore(ServiceParameterPersister.java:155)

at net.refractions.udig.catalog.internal.CatalogImpl.loadFromFile(CatalogImpl.java:1015)

at net.refractions.udig.catalog.CatalogPlugin.restoreFromPreferences(CatalogPlugin.java:201)

at net.refractions.udig.catalog.CatalogPlugin.start(CatalogPlugin.java:106)

I am going to think about a fix for that; it could be our habit of asking for getInfo to check if the service is any good is:

a) a bit of a hack

b) not needed when restoring the catalog no startup

-- 

Jody Garnett