Good catch :) . Gayan can you please look into such type of errors inside user.core component.
Thanks. On Fri, Feb 20, 2015 at 5:45 PM, Sajith Ariyarathna <[email protected]> wrote: > I think I have found a bug in JDBCUserStoreManager class at persistUser > <https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java#L1159> > method. > > In line 1166 > <https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java#L1166> > a > database connection is created by calling the getDBConnection() > <https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java#L976> > method which throws SQLException and UserStoreException. If it throws an > exception, catch clause [ catch (Throwable e) ] in line 1263 > <https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java#L1263> > will catch it and tries to rollback the database connection. > But dbConnection is null at this moment, and it will result a > NullPointerException; which is not handled. > > My suggestion to fix this bug is to handle the exceptions of creating the > database connection (at line 1166 > <https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java#L1166>) > in a separate try-catch block. > > > On Fri, Feb 20, 2015 at 10:39 AM, Udara Liyanage <[email protected]> wrote: > >> +1 for fixing, catching Throwable is not a good practice unless it is >> highly needed >> >> On Thu, Feb 19, 2015 at 6:07 PM, Johann Nallathamby <[email protected]> >> wrote: >> >>> I can't see any particular reason. +1 to fix it. >>> >>> On Thu, Feb 19, 2015 at 4:28 PM, Gayan Gunawardana <[email protected]> >>> wrote: >>> >>>> Hi, >>>> >>>> In user core [1] JDBCUserStoreManager, persistUser user method >>>> contains catching Throwable. In general catching Throwable is not a >>>> good practice because there may be some run time Errors like >>>> OutOfMemoryError >>>> which are not be able to handle. Is there any particular reason to catch >>>> Throwable? >>>> >>>> [1] >>>> https://github.com/wso2/carbon4-kernel/blob/master/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/jdbc/JDBCUserStoreManager.java >>>> -- >>>> Gayan Gunawardana >>>> Software Engineer; WSO2 Inc.; http://wso2.com/ >>>> Email: [email protected] >>>> Mobile: +94 (71) 8020933 >>>> >>> >>> >>> >>> -- >>> Thanks & Regards, >>> >>> *Johann Dilantha Nallathamby* >>> Associate Technical Lead & Product Lead of WSO2 Identity Server >>> Integration Technologies Team >>> WSO2, Inc. >>> lean.enterprise.middleware >>> >>> Mobile - *+94777776950* >>> Blog - *http://nallaa.wordpress.com <http://nallaa.wordpress.com>* >>> >>> _______________________________________________ >>> Dev mailing list >>> [email protected] >>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>> >>> >> >> >> -- >> >> Udara Liyanage >> Software Engineer >> WSO2, Inc.: http://wso2.com >> lean. enterprise. middleware >> >> web: http://udaraliyanage.wordpress.com >> phone: +94 71 443 6897 >> >> _______________________________________________ >> Dev mailing list >> [email protected] >> http://wso2.org/cgi-bin/mailman/listinfo/dev >> >> > > > -- > Sajith Ariyarathna > Software Engineer; WSO2, Inc.; http://wso2.com/ > mobile: +94 77 6602284, +94 71 3951048 > -- Thanks & Regards, *Johann Dilantha Nallathamby* Associate Technical Lead & Product Lead of WSO2 Identity Server Integration Technologies Team WSO2, Inc. lean.enterprise.middleware Mobile - *+94777776950* Blog - *http://nallaa.wordpress.com <http://nallaa.wordpress.com>*
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
