On Fri, Sep 12, 2014 at 7:55 AM, Nipuni Perera <nip...@wso2.com> wrote:
> Hi, > > Please review and merge pull request[1] for the above null check. > > [1] https://github.com/wso2-dev/carbon-registry/pull/22 > +1. That is summary of mail thread ;) -Ajith. > > > Thanks, > Nipuni > > On Wed, Sep 10, 2014 at 5:18 PM, Lasantha Fernando <lasan...@wso2.com> > wrote: > >> >> >> On 10 September 2014 16:55, Nipuni Perera <nip...@wso2.com> 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 <lasan...@wso2.com> >>> 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 <aji...@wso2.com> 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 <nip...@wso2.com> >>>>> 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 <firz...@wso2.com> >>>>>> 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 < >>>>>>> lasan...@wso2.com> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> >>>>>>>> On 10 September 2014 10:19, Nipuni Perera <nip...@wso2.com> 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 <kas...@wso2.com >>>>>>>>> > 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 <nip...@wso2.com> >>>>>>>>>> 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: nip...@wso2.com >>>>>>>>>>> Git hub profile: https://github.com/nipuni >>>>>>>>>>> Mobile: +94 (71) 5626680 >>>>>>>>>>> <http://wso2.com> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> _______________________________________________ >>>>>>>>>>> Dev mailing list >>>>>>>>>>> Dev@wso2.org >>>>>>>>>>> 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: nip...@wso2.com >>>>>>>>> Git hub profile: https://github.com/nipuni >>>>>>>>> Mobile: +94 (71) 5626680 >>>>>>>>> <http://wso2.com> >>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> Dev mailing list >>>>>>>>> Dev@wso2.org >>>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> *Lasantha Fernando* >>>>>>>> Software Engineer - Data Technologies Team >>>>>>>> WSO2 Inc. http://wso2.com >>>>>>>> >>>>>>>> email: lasan...@wso2.com >>>>>>>> mobile: (+94) 71 5247551 >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Dev mailing list >>>>>>>> Dev@wso2.org >>>>>>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Nipuni Perera >>>>>> Software Engineer; WSO2 Inc.; http://wso2.com >>>>>> Email: nip...@wso2.com >>>>>> Git hub profile: https://github.com/nipuni >>>>>> Mobile: +94 (71) 5626680 >>>>>> <http://wso2.com> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Ajith Vitharana. >>>>> WSO2 Inc. - http://wso2.org >>>>> Email : aji...@wso2.com >>>>> Mobile : +94772217350 >>>>> >>>>> >>>> >>>> >>>> -- >>>> *Lasantha Fernando* >>>> Software Engineer - Data Technologies Team >>>> WSO2 Inc. http://wso2.com >>>> >>>> email: lasan...@wso2.com >>>> mobile: (+94) 71 5247551 >>>> >>> >>> >>> >>> -- >>> Nipuni Perera >>> Software Engineer; WSO2 Inc.; http://wso2.com >>> Email: nip...@wso2.com >>> 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: lasan...@wso2.com >> mobile: (+94) 71 5247551 >> > > > > -- > Nipuni Perera > Software Engineer; WSO2 Inc.; http://wso2.com > Email: nip...@wso2.com > Git hub profile: https://github.com/nipuni > Mobile: +94 (71) 5626680 > <http://wso2.com> > > > _______________________________________________ > Dev mailing list > Dev@wso2.org > http://wso2.org/cgi-bin/mailman/listinfo/dev > > -- Ajith Vitharana. WSO2 Inc. - http://wso2.org Email : aji...@wso2.com Mobile : +94772217350
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
