You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/09/09 16:09:31 UTC

[GitHub] [shardingsphere] yx9o opened a new pull request #12324: Improve resource release logic to prevent memory leaks.

yx9o opened a new pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324


   Improve resource release logic to prevent memory leaks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] terrymanu merged pull request #12324: Improve resource release logic to prevent memory leaks.

Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] codecov-commenter commented on pull request #12324: Improve resource release logic to prevent memory leaks.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324#issuecomment-916431101


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#12324](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (24bb684) into [master](https://codecov.io/gh/apache/shardingsphere/commit/825e29e601dea8ac8131a30de5ceefbf71752150?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (825e29e) will **decrease** coverage by `0.00%`.
   > The diff coverage is `98.90%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/12324/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #12324      +/-   ##
   ============================================
   - Coverage     63.83%   63.82%   -0.01%     
     Complexity     1272     1272              
   ============================================
     Files          2365     2365              
     Lines         35880    35884       +4     
     Branches       6232     6218      -14     
   ============================================
   - Hits          22903    22902       -1     
   - Misses        11132    11157      +25     
   + Partials       1845     1825      -20     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...d/query/binary/bind/PostgreSQLComBindExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9jb21tYW5kL3F1ZXJ5L2JpbmFyeS9iaW5kL1Bvc3RncmVTUUxDb21CaW5kRXhlY3V0b3IuamF2YQ==) | `33.33% <0.00%> (ø)` | |
   | [...tasource/creator/impl/HikariDataSourceCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9jb25maWcvZGF0YXNvdXJjZS9jcmVhdG9yL2ltcGwvSGlrYXJpRGF0YVNvdXJjZUNyZWF0b3IuamF2YQ==) | `85.00% <100.00%> (+3.75%)` | :arrow_up: |
   | [...natived/builder/dialect/MySQLPrivilegeHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWF1dGhvcml0eS9zaGFyZGluZ3NwaGVyZS1hdXRob3JpdHktY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYXV0aG9yaXR5L3Byb3ZpZGVyL25hdGl2ZWQvYnVpbGRlci9kaWFsZWN0L015U1FMUHJpdmlsZWdlSGFuZGxlci5qYXZh) | `82.97% <100.00%> (+2.83%)` | :arrow_up: |
   | [...atived/builder/dialect/OraclePrivilegeHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWF1dGhvcml0eS9zaGFyZGluZ3NwaGVyZS1hdXRob3JpdHktY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYXV0aG9yaXR5L3Byb3ZpZGVyL25hdGl2ZWQvYnVpbGRlci9kaWFsZWN0L09yYWNsZVByaXZpbGVnZUhhbmRsZXIuamF2YQ==) | `82.69% <100.00%> (+2.88%)` | :arrow_up: |
   | [...ed/builder/dialect/PostgreSQLPrivilegeHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWF1dGhvcml0eS9zaGFyZGluZ3NwaGVyZS1hdXRob3JpdHktY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYXV0aG9yaXR5L3Byb3ZpZGVyL25hdGl2ZWQvYnVpbGRlci9kaWFsZWN0L1Bvc3RncmVTUUxQcml2aWxlZ2VIYW5kbGVyLmphdmE=) | `85.98% <100.00%> (+2.80%)` | :arrow_up: |
   | [...ved/builder/dialect/SQLServerPrivilegeHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWF1dGhvcml0eS9zaGFyZGluZ3NwaGVyZS1hdXRob3JpdHktY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYXV0aG9yaXR5L3Byb3ZpZGVyL25hdGl2ZWQvYnVpbGRlci9kaWFsZWN0L1NRTFNlcnZlclByaXZpbGVnZUhhbmRsZXIuamF2YQ==) | `64.84% <100.00%> (+2.42%)` | :arrow_up: |
   | [...command/query/text/PostgreSQLComQueryExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9jb21tYW5kL3F1ZXJ5L3RleHQvUG9zdGdyZVNRTENvbVF1ZXJ5RXhlY3V0b3IuamF2YQ==) | `82.14% <100.00%> (ø)` | |
   | [...sphere/scaling/core/job/schedule/JobScheduler.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9qb2Ivc2NoZWR1bGUvSm9iU2NoZWR1bGVyLmphdmE=) | `20.00% <0.00%> (-26.67%)` | :arrow_down: |
   | [...re/scaling/core/executor/engine/ExecuteEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9leGVjdXRvci9lbmdpbmUvRXhlY3V0ZUVuZ2luZS5qYXZh) | `88.88% <0.00%> (-11.12%)` | :arrow_down: |
   | [...scaling/core/job/task/inventory/InventoryTask.java](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc2NhbGluZy9zaGFyZGluZ3NwaGVyZS1zY2FsaW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvY29yZS9qb2IvdGFzay9pbnZlbnRvcnkvSW52ZW50b3J5VGFzay5qYXZh) | `68.88% <0.00%> (-6.67%)` | :arrow_down: |
   | ... and [65 more](https://codecov.io/gh/apache/shardingsphere/pull/12324/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e972b34...24bb684](https://codecov.io/gh/apache/shardingsphere/pull/12324?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] yx9o commented on a change in pull request #12324: Improve resource release logic to prevent memory leaks.

Posted by GitBox <gi...@apache.org>.
yx9o commented on a change in pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324#discussion_r705971659



##########
File path: shardingsphere-kernel/shardingsphere-authority/shardingsphere-authority-core/src/main/java/org/apache/shardingsphere/authority/provider/natived/builder/dialect/MySQLPrivilegeHandler.java
##########
@@ -56,12 +56,13 @@
     @Override
     public Collection<ShardingSphereUser> diff(final Collection<ShardingSphereUser> users, final DataSource dataSource) throws SQLException {
         Collection<Grantee> grantees = new LinkedList<>();
-        try (Connection connection = dataSource.getConnection()) {
-            Statement statement = connection.createStatement();
-            try (ResultSet resultSet = statement.executeQuery(getGlobalPrivilegesSQL(users))) {
-                while (resultSet.next()) {
-                    grantees.add(new Grantee(resultSet.getString("user"), resultSet.getString("host")));
-                }
+        try (
+                Connection connection = dataSource.getConnection();
+                Statement statement = connection.createStatement();
+                ResultSet resultSet = statement.executeQuery(getGlobalPrivilegesSQL(users))
+        ) {
+            while (resultSet.next()) {
+                grantees.add(new Grantee(resultSet.getString("user"), resultSet.getString("host")));

Review comment:
       > Why the original code may resource leak?
   > The connection, statement and resultSet are all in try with resource
   
   `Statement statement = connection.createStatement();` not in try with resource.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] yx9o commented on pull request #12324: Improve resource release logic to prevent memory leaks.

Posted by GitBox <gi...@apache.org>.
yx9o commented on pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324#issuecomment-922498475


   @terrymanu Hi, please review, thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] terrymanu commented on a change in pull request #12324: Improve resource release logic to prevent memory leaks.

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12324:
URL: https://github.com/apache/shardingsphere/pull/12324#discussion_r705967916



##########
File path: shardingsphere-kernel/shardingsphere-authority/shardingsphere-authority-core/src/main/java/org/apache/shardingsphere/authority/provider/natived/builder/dialect/MySQLPrivilegeHandler.java
##########
@@ -56,12 +56,13 @@
     @Override
     public Collection<ShardingSphereUser> diff(final Collection<ShardingSphereUser> users, final DataSource dataSource) throws SQLException {
         Collection<Grantee> grantees = new LinkedList<>();
-        try (Connection connection = dataSource.getConnection()) {
-            Statement statement = connection.createStatement();
-            try (ResultSet resultSet = statement.executeQuery(getGlobalPrivilegesSQL(users))) {
-                while (resultSet.next()) {
-                    grantees.add(new Grantee(resultSet.getString("user"), resultSet.getString("host")));
-                }
+        try (
+                Connection connection = dataSource.getConnection();
+                Statement statement = connection.createStatement();
+                ResultSet resultSet = statement.executeQuery(getGlobalPrivilegesSQL(users))
+        ) {
+            while (resultSet.next()) {
+                grantees.add(new Grantee(resultSet.getString("user"), resultSet.getString("host")));

Review comment:
       Why the original code may resource leak?
   The connection, statement and resultSet are all in try with resource




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org