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 2022/07/21 03:28:00 UTC
[GitHub] [shardingsphere] azexcy opened a new pull request, #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
azexcy opened a new pull request, #19410:
URL: https://github.com/apache/shardingsphere/pull/19410
Fixes #19183.
Changes proposed in this pull request:
- Release newly ByteBuf
--
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 #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #19410:
URL: https://github.com/apache/shardingsphere/pull/19410#issuecomment-1191019035
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/19410?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 [#19410](https://codecov.io/gh/apache/shardingsphere/pull/19410?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f13c8ef) into [master](https://codecov.io/gh/apache/shardingsphere/commit/7c76b52c2268e663dc83d9e56e939d98aafc97c1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c76b52) will **decrease** coverage by `0.04%`.
> The diff coverage is `58.55%`.
```diff
@@ Coverage Diff @@
## master #19410 +/- ##
============================================
- Coverage 59.68% 59.63% -0.05%
- Complexity 2373 2378 +5
============================================
Files 3819 3823 +4
Lines 54382 54439 +57
Branches 7629 7637 +8
============================================
+ Hits 32456 32464 +8
- Misses 19122 19180 +58
+ Partials 2804 2795 -9
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/19410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/ShardingRuleAlteredJobConfigurationPreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvZGF0YS9waXBlbGluZS9TaGFyZGluZ1J1bGVBbHRlcmVkSm9iQ29uZmlndXJhdGlvblByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...e/infra/config/props/ConfigurationPropertyKey.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9jb25maWcvcHJvcHMvQ29uZmlndXJhdGlvblByb3BlcnR5S2V5LmphdmE=) | `100.00% <ø> (ø)` | |
| [...e/schema/decorator/model/ShardingSphereSchema.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9kYXRhYmFzZS9zY2hlbWEvZGVjb3JhdG9yL21vZGVsL1NoYXJkaW5nU3BoZXJlU2NoZW1hLmphdmE=) | `71.42% <0.00%> (-3.58%)` | :arrow_down: |
| [.../executor/advanced/AdvancedFederationExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZmVkZXJhdGlvbi9zaGFyZGluZ3NwaGVyZS1pbmZyYS1mZWRlcmF0aW9uLWV4ZWN1dG9yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9mZWRlcmF0aW9uL2V4ZWN1dG9yL2FkdmFuY2VkL0FkdmFuY2VkRmVkZXJhdGlvbkV4ZWN1dG9yLmphdmE=) | `25.00% <0.00%> (ø)` | |
| [.../executor/original/OriginalFederationExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZmVkZXJhdGlvbi9zaGFyZGluZ3NwaGVyZS1pbmZyYS1mZWRlcmF0aW9uLWV4ZWN1dG9yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9mZWRlcmF0aW9uL2V4ZWN1dG9yL29yaWdpbmFsL09yaWdpbmFsRmVkZXJhdGlvbkV4ZWN1dG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...or/original/table/FilterableTableScanExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZmVkZXJhdGlvbi9zaGFyZGluZ3NwaGVyZS1pbmZyYS1mZWRlcmF0aW9uLWV4ZWN1dG9yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9mZWRlcmF0aW9uL2V4ZWN1dG9yL29yaWdpbmFsL3RhYmxlL0ZpbHRlcmFibGVUYWJsZVNjYW5FeGVjdXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../driver/jdbc/adapter/AbstractStatementAdapter.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtamRiYy9zaGFyZGluZ3NwaGVyZS1qZGJjLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RyaXZlci9qZGJjL2FkYXB0ZXIvQWJzdHJhY3RTdGF0ZW1lbnRBZGFwdGVyLmphdmE=) | `72.85% <0.00%> (ø)` | |
| [...core/check/consistency/DataConsistencyChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvY2hlY2svY29uc2lzdGVuY3kvRGF0YUNvbnNpc3RlbmN5Q2hlY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../core/metadata/generator/PipelineDDLGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvbWV0YWRhdGEvZ2VuZXJhdG9yL1BpcGVsaW5lRERMR2VuZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../proxy/backend/communication/ProxySQLExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/19410/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC9jb21tdW5pY2F0aW9uL1Byb3h5U1FMRXhlY3V0b3IuamF2YQ==) | `29.09% <0.00%> (ø)` | |
| ... and [81 more](https://codecov.io/gh/apache/shardingsphere/pull/19410/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) | |
--
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] sandynz commented on a diff in pull request #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #19410:
URL: https://github.com/apache/shardingsphere/pull/19410#discussion_r926280616
##########
shardingsphere-db-protocol/shardingsphere-db-protocol-mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/binlog/row/column/value/string/MySQLJsonBinlogProtocolValue.java:
##########
@@ -39,7 +40,10 @@ public final class MySQLJsonBinlogProtocolValue implements MySQLBinlogProtocolVa
@Override
public Serializable read(final MySQLBinlogColumnDef columnDef, final MySQLPacketPayload payload) {
- return MySQLJsonValueDecoder.decode(payload.getByteBuf().readBytes(readLengthFromMeta(columnDef.getColumnMeta(), payload)));
+ ByteBuf newByteBuf = payload.getByteBuf().readBytes(readLengthFromMeta(columnDef.getColumnMeta(), payload));
+ Serializable result = MySQLJsonValueDecoder.decode(newByteBuf);
+ newByteBuf.release();
+ return result;
Review Comment:
It's better to do release() in finally block if we keep allocating a new ByteBuf, since MySQLJsonValueDecoder.decode might throw exception.
--
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] azexcy commented on pull request #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
Posted by GitBox <gi...@apache.org>.
azexcy commented on PR #19410:
URL: https://github.com/apache/shardingsphere/pull/19410#issuecomment-1191003868
The implementation of readBytes(int length) is at `io.netty.buffer.AbstractByteBuf`
```
@Override
public ByteBuf readBytes(int length) {
checkReadableBytes(length);
if (length == 0) {
return Unpooled.EMPTY_BUFFER;
}
ByteBuf buf = alloc().buffer(length, maxCapacity);
buf.writeBytes(this, readerIndex, length);
readerIndex += length;
return buf;
}
```
--
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] azexcy commented on a diff in pull request #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #19410:
URL: https://github.com/apache/shardingsphere/pull/19410#discussion_r926311494
##########
shardingsphere-db-protocol/shardingsphere-db-protocol-mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/binlog/row/column/value/string/MySQLJsonBinlogProtocolValue.java:
##########
@@ -39,7 +40,10 @@ public final class MySQLJsonBinlogProtocolValue implements MySQLBinlogProtocolVa
@Override
public Serializable read(final MySQLBinlogColumnDef columnDef, final MySQLPacketPayload payload) {
- return MySQLJsonValueDecoder.decode(payload.getByteBuf().readBytes(readLengthFromMeta(columnDef.getColumnMeta(), payload)));
+ ByteBuf newByteBuf = payload.getByteBuf().readBytes(readLengthFromMeta(columnDef.getColumnMeta(), payload));
+ Serializable result = MySQLJsonValueDecoder.decode(newByteBuf);
+ newByteBuf.release();
+ return result;
Review Comment:
Fixed
--
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] sandynz merged pull request #19410: Fix MySQLJsonBinlogProtocolValue may cause leak
Posted by GitBox <gi...@apache.org>.
sandynz merged PR #19410:
URL: https://github.com/apache/shardingsphere/pull/19410
--
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