Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: fix StringUtils StackOverflowError #3828

Merged
merged 21 commits into from
Jun 17, 2021

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Jun 15, 2021

bugfix: fix StringUtils StackOverflowError.

fixed: #3826

@wangliang181230 wangliang181230 changed the title bugfix: StringUtils StackOverflowError bugfix: fix StringUtils StackOverflowError Jun 15, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2021

Codecov Report

Merging #3828 (f78c14f) into develop (9e5a131) will increase coverage by 0.23%.
The diff coverage is 81.25%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3828      +/-   ##
=============================================
+ Coverage      40.67%   40.91%   +0.23%     
- Complexity      2965     2985      +20     
=============================================
  Files            664      665       +1     
  Lines          22418    22465      +47     
  Branches        2781     2794      +13     
=============================================
+ Hits            9118     9191      +73     
+ Misses         12453    12427      -26     
  Partials         847      847              
Impacted Files Coverage Δ
...io/seata/rm/tcc/interceptor/ActionContextUtil.java 37.34% <0.00%> (ø)
...a/io/seata/common/util/CycleDependencyHandler.java 75.86% <75.86%> (ø)
...rc/main/java/io/seata/common/util/StringUtils.java 26.57% <77.27%> (+10.41%) ⬆️
...ain/java/io/seata/common/util/CollectionUtils.java 92.00% <94.44%> (ø)
...main/java/io/seata/common/util/ReflectionUtil.java 94.59% <100.00%> (ø)
...tasource/sql/struct/cache/MysqlTableMetaCache.java 81.60% <0.00%> (+1.14%) ⬆️
...io/seata/core/rpc/netty/AbstractNettyRemoting.java 14.76% <0.00%> (+1.34%) ⬆️
...main/java/io/seata/common/holder/ObjectHolder.java 50.00% <0.00%> (+50.00%) ⬆️

@wangliang181230 wangliang181230 added this to the 1.5.0 milestone Jun 15, 2021
@jsbxyyx
Copy link
Member

jsbxyyx commented Jun 16, 2021

public class AClass {
        private AClass a=new AClass();
        private AClass b=new AClass();
        public String toString(){
            return StringUtils.toString(this);
        }
}

如果caller是toString无参方法,则直接忽略?

@wangliang181230
Copy link
Contributor Author

public class AClass {
        private AClass a=new AClass();
        private AClass b=new AClass();
        public String toString(){
            return StringUtils.toString(this);
        }
}

如果caller是toString无参方法,则直接忽略?

这个类本身就无法实例化,new AClass()时直接StackOverflowError了,你可以试一下看看。

另外,针对toString()里调StringUtils.toString(Object obj)方法的问题。目前的代码逻辑不会去调自定义类型的toString()方法的。会一直解析,直到读取到简单类型(CharSequence、Number、Boolean、Character、Date、Enum),将简单类型直接转换成字符串。
这个是早上我刚处理的,你重新拉取一下代码看一下吧。

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Jun 16, 2021

另外,关于循环引用的问题,目前还是存在的,如果不是引用在第一层,而是嵌套了多层的话。
我计划建一个循环引用处理器,专门处理这个情况。

@wangliang181230
Copy link
Contributor Author

另外,关于循环引用的问题,目前还是存在的,如果不是引用在第一层,而是嵌套了多层的话。
我计划建一个循环引用处理器,专门处理这个情况。

@jsbxyyx done, 一级循环引用,以及多级循环引用,都已解决。
具体解决情况,请查看StringUtilsTest中的测试用例。

*/
private static Object getObjectUniqueCode(Object obj) {
return System.identityHashCode(obj);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object getUniqueSubstituteObject(Object obj)方法,用来代替obj保存到Set中,避免obj.hashCode()方法,在objCollectionMap且存在循环引用时,直接抛出StackOverflowError异常。

@jsbxyyx
Copy link
Member

jsbxyyx commented Jun 16, 2021

public class AClass {
        private AClass a=new AClass();
        private AClass b=new AClass();
        public String toString(){
            return StringUtils.toString(this);
        }
}

如果caller是toString无参方法,则直接忽略?

这个类本身就无法实例化,new AClass()时直接StackOverflowError了,你可以试一下看看。

另外,针对toString()里调StringUtils.toString(Object obj)方法的问题。目前的代码逻辑不会去调自定义类型的toString()方法的。会一直解析,直到读取到简单类型(CharSequence、Number、Boolean、Character、Date、Enum),将简单类型直接转换成字符串。
这个是早上我刚处理的,你重新拉取一下代码看一下吧。

我的问题

Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@caohdgege caohdgege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, 请补充一下md文件

@wangliang181230
Copy link
Contributor Author

LGTM, 请补充一下md文件

done

@caohdgege caohdgege added the module/core core module label Jun 17, 2021
@caohdgege caohdgege merged commit 25385b3 into apache:develop Jun 17, 2021
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wangliang181230 wangliang181230 deleted the optimize/toString-util branch June 17, 2021 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/core core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

io.seata.common.util.StringUtils#toString可能会导致StackOverFlowError
5 participants