Hi,

Looks like we haven't still merged the pull request. This issue affects AS
as well. Can we pls merge this.. ?

thanks,

On Fri, Sep 12, 2014 at 3:31 PM, Ajith Vitharana <aji...@wso2.com> wrote:

>
>
> 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
>
>


-- 
Supun Malinga,

Senior Software Engineer,
WSO2 Inc.
http://wso2.com
email: sup...@wso2.com <sup...@wso2.com>
mobile: +94 (0)71 56 91 321
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to