Hi, Please review and merge pull request[1] for the above null check.
[1] https://github.com/wso2-dev/carbon-registry/pull/22 Thanks, Nipuni On Wed, Sep 10, 2014 at 5:18 PM, Lasantha Fernando <[email protected]> wrote: > > > On 10 September 2014 16:55, Nipuni Perera <[email protected]> wrote: > >> Hi Lasantha, >> >> I have compared AS 5.2.1 and AS 5.3.0-SNAPSHOT versions with osgi console >> enabled. I could see that the following 2 bundles are in unsatisfied state >> in 5.2.1 server while they are in active state in 5.3.0-SNAPSHOT. (AS does >> not use indexing feature hence does not has indexingConfiguration entry in >> registry.xml. ) >> >> Unsatisfied org.wso2.carbon.registry.indexing >> org.wso2.carbon.registry.indexing(bid=340) >> Unsatisfied registry.search.dscomponent >> org.wso2.carbon.registry.search(bid=347) >> >> When server is shutting down, services list in >> waitForServerTaskCompletion() in ServerManagement.java [1] (line 110) >> contains 2 services in 5.3.0-SNAPSHOT (RegistryCoreServiceComponent and >> IndexingServiceComponent) and in 5.2.1 pack list only one >> (RegistryCoreServiceComponent). The NPE occurs when shutting down the >> service "IndexingServiceComponent" in 5.3.0-SNAPSHOT. It seems that the >> unsatisfied bundles in earlier version is fixed in later version. >> > > Oh OK... I thought the swallowed NPE was somehow getting logged during a > graceful restart, which is why I referenced the earlier thread. Seems the > JIRA raised by you is for a different issue. > > As I debug I could see that in the normal startup this NPE is thrown but >> swallowed in 5.2.1. Will debug further and update the thread with the >> findings. >> > > Thanks for looking into this! > > >> >> [1] >> https://github.com/wso2-dev/carbon4-kernel/blob/master/core/org.wso2.carbon.core/src/main/java/org/wso2/carbon/core/ServerManagement.java >> >> Thanks, >> Nipuni >> >> >> On Wed, Sep 10, 2014 at 2:43 PM, Lasantha Fernando <[email protected]> >> wrote: >> >>> Hi Nipuni, >>> >>> Can you also verify if this code segment affects a normal startup as >>> well (other than a graceful restart)? >>> >>> i.e. if u start with OSGi console, you can verify if >>> registry.search.dscomponent service is satisfied or not. Then if you drill >>> down the unsatisfied component, you will probably be able to find the NPE >>> in the reasons listed for OSGi service being unsatisfied. I think that this >>> NPE is thrown at normal startup as well, but somehow swallowed up when the >>> bundle loading happens. I mean that was the case in the issue mentioned in >>> the earlier thread. >>> >>> Thanks, >>> Lasantha >>> >>> On 10 September 2014 14:26, Ajith Vitharana <[email protected]> wrote: >>> >>>> Hi Nipuni, >>>> >>>> You need to do two things here, >>>> >>>> 1. If you are not using the registry search feature , then need to find >>>> the place that bundle going to include to the product. >>>> 2. You can do a null check and proceed in code.[ @Subash this is bug in >>>> registry code please fix.] >>>> >>>> -Ajith. >>>> >>>> >>>> >>>> On Wed, Sep 10, 2014 at 12:47 PM, Nipuni Perera <[email protected]> >>>> wrote: >>>> >>>>> Hi, >>>>> >>>>> Adding null check can avoid the NPE exception but may not solve the >>>>> issue properly. >>>>> >>>>> @Lasantha, >>>>> >>>>> I will check with GREG team and see if this can be solved with >>>>> touchpoints. >>>>> >>>>> @Firzhan. >>>>> >>>>> I have gone through the discussion "[AS] Error in shutting down the >>>>> server : Cannot put transports into maintenance mode : NPE from >>>>> RegistryConfigLoader." earlier and your findings, will check with GREG >>>>> team. >>>>> >>>>> Thanks, >>>>> Nipuni >>>>> >>>>> >>>>> On Wed, Sep 10, 2014 at 11:44 AM, Firzhan Naqash <[email protected]> >>>>> wrote: >>>>> >>>>>> Hi All, >>>>>> >>>>>> We have already discussed this issue in the mail thread [Dev] [AS] >>>>>> Error in shutting down the server : Cannot put transports into >>>>>> maintenance >>>>>> mode : NPE from RegistryConfigLoader. >>>>>> >>>>>> Rather than checking the null point, I feel like we have to >>>>>> investigate why the registry's search feature bundle getting included to >>>>>> the products which are not using registry search feature ( BPS ). >>>>>> >>>>>> @Ajith:- You thoughts on this >>>>>> >>>>>> Regards, >>>>>> Firzhan >>>>>> >>>>>> On Wed, Sep 10, 2014 at 11:41 AM, Lasantha Fernando < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> >>>>>>> On 10 September 2014 10:19, Nipuni Perera <[email protected]> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Yes. I can add a null check to indexingConfig element and continue >>>>>>>> only if the value is available (But it will add the whole method >>>>>>>> execution >>>>>>>> inside constructor RegistryConfigLoader() to an if block - line 75 to >>>>>>>> 150 >>>>>>>> of [1]). Is this preferable? >>>>>>>> >>>>>>>> public RegistryConfigLoader() { >>>>>>>> try { >>>>>>>> FileInputStream fileInputStream = new >>>>>>>> FileInputStream(getConfigFile()); >>>>>>>> StAXOMBuilder builder = new StAXOMBuilder( >>>>>>>> >>>>>>>> CarbonUtils.replaceSystemVariablesInXml(fileInputStream)); >>>>>>>> OMElement configElement = builder.getDocumentElement(); >>>>>>>> OMElement indexingConfig = >>>>>>>> configElement.getFirstChildWithName( >>>>>>>> new QName("indexingConfiguration")); >>>>>>>> if (indexingConfig != null) >>>>>>>> { >>>>>>>> //newly >>>>>>>> added if block >>>>>>>> >>>>>>>> ... >>>>>>>> ... >>>>>>>> } >>>>>>>> } >>>>>>>> } catch (FileNotFoundException e) { >>>>>>>> // This virtually cannot happen as registry.xml is >>>>>>>> necessary to start up the registry >>>>>>>> log.error("registry.xml has not been found", e); >>>>>>>> } catch (RegistryException e) { >>>>>>>> log.error(e.getMessage(),e); >>>>>>>> } catch (XMLStreamException e) { >>>>>>>> String msg = "error building registry.xml, check for >>>>>>>> badly formed xml"; >>>>>>>> log.error(msg, e); >>>>>>>> } catch (CarbonException e) { >>>>>>>> log.error("An error occurred during system variable >>>>>>>> replacement", e); >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> It seems if the indexingConfig is null, no need to call this >>>>>>>> constructor. RegistryConfigLoader() is called inside IndexingManager() >>>>>>>> constructor. >>>>>>>> >>>>>>>> private IndexingManager() { >>>>>>>> try { >>>>>>>> registry = >>>>>>>> Utils.getRegistryService().getRegistry(CarbonConstants.REGISTRY_SYSTEM_USERNAME); >>>>>>>> registryConfig = new RegistryConfigLoader(); >>>>>>>> // call to constructor RegistryConfigLoader() >>>>>>>> indexer = new AsyncIndexer(); >>>>>>>> } catch (RegistryException e) { >>>>>>>> log.error("Could not initialize registry for indexing", >>>>>>>> e); >>>>>>>> } >>>>>>>> } >>>>>>>> Can't we check indexingConfig value before calling >>>>>>>> RegistryConfigLoader() ? >>>>>>>> >>>>>>> >>>>>>> +1 to add a null check and deal with this. As to not calling >>>>>>> RegistryConfigLoader if indexingConfig is not present, isn't >>>>>>> RegistryConfigLoader there to read the whole registry.xml file and >>>>>>> indexingConfig is only a part of that? If so, I guess we do need to >>>>>>> call >>>>>>> RegistryConfigLoader() regardless of whether indexingConfig is present >>>>>>> or >>>>>>> not. Maybe the GREG team can give better input on this. >>>>>>> >>>>>>> Also, this exact same code segment was discussed before in thread "[Dev] >>>>>>> Missing configuration in registry.xml causes OSGi services to be >>>>>>> unsatisfied" [1], but guess I forgot to raise a JIRA under that... >>>>>>> :-). Can you look into that thread as well and if it is possible to add >>>>>>> a >>>>>>> touchpoint as well as suggested in that thread? >>>>>>> >>>>>>> [1] http://mail.wso2.org/mailarchive/dev/2014-April/029358.html >>>>>>> >>>>>>> Thanks, >>>>>>> Lasantha >>>>>>> >>>>>>>> >>>>>>>> [1] >>>>>>>> https://github.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Nipuni >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Sep 10, 2014 at 8:53 AM, Kasun Gajasinghe <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> As a best practice, do not catch the Exception class, instead >>>>>>>>> catch specific subclasses. But, you do not need to catch any >>>>>>>>> exceptions in >>>>>>>>> this particular case? Can't you just verify whether the >>>>>>>>> indexingConfiguration >>>>>>>>> element is available or not? >>>>>>>>> >>>>>>>>> KasunG >>>>>>>>> >>>>>>>>> On Wed, Sep 10, 2014 at 4:39 AM, Nipuni Perera <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> I am working on issue[1]. I could find that the >>>>>>>>>> RegistryConfigLoader() returns a null value. (line 73 in [2]). >>>>>>>>>> >>>>>>>>>> OMElement indexingConfig = >>>>>>>>>> configElement.getFirstChildWithName(new >>>>>>>>>> QName("indexingConfiguration")); >>>>>>>>>> >>>>>>>>>> This value is used in several places. but NPE is thrown in line >>>>>>>>>> 88, >>>>>>>>>> >>>>>>>>>> lastAccessTimeLocation = >>>>>>>>>> indexingConfig.getFirstChildWithName(new >>>>>>>>>> QName("lastAccessTimeLocation")).getText(); >>>>>>>>>> >>>>>>>>>> as the catch clause catches only >>>>>>>>>> >>>>>>>>>> catch (OMException e) { >>>>>>>>>> ... // set default value >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> while in other places (eg: line 78,79) it is >>>>>>>>>> >>>>>>>>>> catch (Exception e) { >>>>>>>>>> ... // set default value >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> This exception occurred due to a missing XML elements in >>>>>>>>>> registry.xml. This missing element ( <indexingConfiguration> >>>>>>>>>> </indexingConfiguration> ) will be used if the product is using >>>>>>>>>> registry >>>>>>>>>> search feature. But this NPE should be handled for the products that >>>>>>>>>> does >>>>>>>>>> not use this feature. This can be fixed adding similar catch clause >>>>>>>>>> (catching Exception e ) in the catch clause(line 90,91) (current >>>>>>>>>> implementation use catch clauses to set default values if an >>>>>>>>>> exception >>>>>>>>>> occurs. Adding an extra catch clause to the later try block can be >>>>>>>>>> used to >>>>>>>>>> add similar behavior). Is there a better way to handle this? >>>>>>>>>> >>>>>>>>>> [1] https://wso2.org/jira/browse/CARBON-14904 >>>>>>>>>> [2] >>>>>>>>>> https://githubc.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java >>>>>>>>>> <https://github.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java> >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Nipuni >>>>>>>>>> -- >>>>>>>>>> Nipuni Perera >>>>>>>>>> Software Engineer; WSO2 Inc.; http://wso2.com >>>>>>>>>> Email: [email protected] >>>>>>>>>> Git hub profile: https://github.com/nipuni >>>>>>>>>> Mobile: +94 (71) 5626680 >>>>>>>>>> <http://wso2.com> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> _______________________________________________ >>>>>>>>>> Dev mailing list >>>>>>>>>> [email protected] >>>>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> *Kasun Gajasinghe*Senior Software Engineer, WSO2 Inc. >>>>>>>>> email: kasung AT spamfree wso2.com >>>>>>>>> linked-in: http://lk.linkedin.com/in/gajasinghe >>>>>>>>> blog: http://kasunbg.org >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Nipuni Perera >>>>>>>> Software Engineer; WSO2 Inc.; http://wso2.com >>>>>>>> Email: [email protected] >>>>>>>> Git hub profile: https://github.com/nipuni >>>>>>>> Mobile: +94 (71) 5626680 >>>>>>>> <http://wso2.com> >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Dev mailing list >>>>>>>> [email protected] >>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *Lasantha Fernando* >>>>>>> Software Engineer - Data Technologies Team >>>>>>> WSO2 Inc. http://wso2.com >>>>>>> >>>>>>> email: [email protected] >>>>>>> mobile: (+94) 71 5247551 >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Dev mailing list >>>>>>> [email protected] >>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Nipuni Perera >>>>> Software Engineer; WSO2 Inc.; http://wso2.com >>>>> Email: [email protected] >>>>> Git hub profile: https://github.com/nipuni >>>>> Mobile: +94 (71) 5626680 >>>>> <http://wso2.com> >>>>> >>>>> >>>> >>>> >>>> -- >>>> Ajith Vitharana. >>>> WSO2 Inc. - http://wso2.org >>>> Email : [email protected] >>>> Mobile : +94772217350 >>>> >>>> >>> >>> >>> -- >>> *Lasantha Fernando* >>> Software Engineer - Data Technologies Team >>> WSO2 Inc. http://wso2.com >>> >>> email: [email protected] >>> mobile: (+94) 71 5247551 >>> >> >> >> >> -- >> Nipuni Perera >> Software Engineer; WSO2 Inc.; http://wso2.com >> Email: [email protected] >> Git hub profile: https://github.com/nipuni >> Mobile: +94 (71) 5626680 >> <http://wso2.com> >> >> > > > -- > *Lasantha Fernando* > Software Engineer - Data Technologies Team > WSO2 Inc. http://wso2.com > > email: [email protected] > mobile: (+94) 71 5247551 > -- Nipuni Perera Software Engineer; WSO2 Inc.; http://wso2.com Email: [email protected] Git hub profile: https://github.com/nipuni Mobile: +94 (71) 5626680 <http://wso2.com>
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
