[+ dev]
Hi All,
We are maintaining below two methods in [1]. Those methods are expecting
multiple PreparedStatements and one or two ResultSets. IMO it is
conceptually wrong to have multiple PreparedStatements with one or two
ResultSets.
public static void closeAllConnections(Connection dbConnection,
ResultSet rs, PreparedStatement... prepStmts) {
closeResultSet(rs);
closeStatements(prepStmts);
closeConnection(dbConnection);
}
public static void closeAllConnections(Connection dbConnection,
ResultSet rs1, ResultSet rs2,
PreparedStatement... prepStmts) {
closeResultSet(rs1);
closeResultSet(rs2);
closeStatements(prepStmts);
closeConnection(dbConnection);
}
In the references of this method [2], we have assigned multiple
PreparedStatement execution results to a single ResultSet. (without closing
the resultset we have re-used it). This is useless and it can cause to a
memory leak as well.
So IMO we should depreciate using above two methods and introduce a new
method to close connections.
[1] https://github.com/wso2-support/carbon4-kernel/blob/
support-4.4.11/core/org.wso2.carbon.user.core/src/main/
java/org/wso2/carbon/user/core/util/DatabaseUtil.java
[2] https://github.com/wso2-support/carbon4-kernel/blob/
support-4.4.11/core/org.wso2.carbon.user.core/src/main/
java/org/wso2/carbon/user/core/authorization/PermissionTree.java#L1012
WDYT?
Thanks
Hasanthi Dissanayake
Software Engineer | WSO2
E: hasan...@wso2.com
M :0718407133| http://wso2.com <http://wso2.com/>
On Tue, Apr 25, 2017 at 4:58 PM, Ruwan Abeykoon <ruw...@wso2.com> wrote:
> Hi All,
> I think we should mark these methods as Deprecated and remove all
> references from IS and user-code side. They promote careless mistakes,
> which are difficult to detect by human or automated tools.
>
> public static void closeAllConnections(Connection dbConnection,
> PreparedStatement... prepStmts) {
>
> public static void closeAllConnections(Connection dbConnection, ResultSet rs,
> PreparedStatement... prepStmts) {
>
> public static void closeAllConnections(Connection dbConnection, ResultSet
> rs1, ResultSet rs2,
> PreparedStatement... prepStmts) {
>
>
> Also we should be able to rewrite the code to use
> newer AutoCloseable thing with java7 for IS 5.3.0+.
>
> Cheers,
> Ruwan
>
> On Tue, Apr 25, 2017 at 4:37 PM, Hasanthi Purnima Dissanayake <
> hasan...@wso2.com> wrote:
>
>> Hi All,
>>
>> We are maintaining below two methods in [1]. Those methods are expecting
>> multiple PreparedStatements and one or two ResultSets. IMO it is
>> conceptually wrong to have multiple PreparedStatements with one or two
>> ResultSets.
>>
>> public static void closeAllConnections(Connection dbConnection, ResultSet
>> rs, PreparedStatement... prepStmts) {
>>
>> closeResultSet(rs);
>> closeStatements(prepStmts);
>> closeConnection(dbConnection);
>> }
>>
>> public static void closeAllConnections(Connection dbConnection, ResultSet
>> rs1, ResultSet rs2,
>> PreparedStatement... prepStmts) {
>> closeResultSet(rs1);
>> closeResultSet(rs2);
>> closeStatements(prepStmts);
>> closeConnection(dbConnection);
>> }
>>
>>
>> In the references of this method [2], we have assigned multiple
>> PreparedStatement execution results to a single ResultSet. (without
>> closing the resultset we have re-used it). This is useless and it can cause
>> to a memory leak as well.
>>
>> So IMO we should depreciate using above two methods and introduce a new
>> method to close connections.
>>
>> [1] https://github.com/wso2-support/carbon4-kernel/blob/support-
>> 4.4.11/core/org.wso2.carbon.user.core/src/main/java/org/
>> wso2/carbon/user/core/util/DatabaseUtil.java
>> [2] https://github.com/wso2-support/carbon4-kernel/blob/support-
>> 4.4.11/core/org.wso2.carbon.user.core/src/main/java/org/
>> wso2/carbon/user/core/authorization/PermissionTree.java#L1012
>>
>>
>> WDYT?
>>
>> Thanks,
>>
>> Hasanthi Dissanayake
>>
>> Software Engineer | WSO2
>>
>> E: hasan...@wso2.com
>> M :0718407133| http://wso2.com <http://wso2.com/>
>>
>
>
>
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev
