-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
[Dubbo-4861] WIP: fix stackoverflow of protostuff and other errors #4862
Conversation
808677c
to
2b0657c
Compare
#3252 your pr and this pr are opposite. I think the two prs are both not the solutions. We should find better solutions. |
I think it's a problem of Protostuff project, it should provide a unified util class to process either loop-reference object or non-loop-reference object. |
yes, I also find this problem. It seems there are something wired about the ProtostuffObjectOutput implement, with this object used, It will cause an endless loop. I will spend more time digging into it. |
@tswstarplanet When I use protostuff directly, There are no endless loop shown up. Maybe this is not the problem of Protostuff? |
@moriadry Good! You can optimize the class |
Codecov Report
@@ Coverage Diff @@
## master #4862 +/- ##
============================================
- Coverage 64.01% 63.89% -0.13%
+ Complexity 453 450 -3
============================================
Files 769 769
Lines 33177 33177
Branches 5229 5229
============================================
- Hits 21239 21199 -40
- Misses 9516 9556 +40
Partials 2422 2422
Continue to review full report at Codecov.
|
I am a little bit confused, @moriadry. What this pull request does is to add more tests, is it sufficient? |
@beiwei30 I caught stackoverflow error when I add two unit tests. And it turns out the Inner class need a Do you think we need to add those two tests in |
More test, better. I will merge. |
What is the purpose of the change
And a test case and fix #4861
Brief changelog
XXXXX
Verifying this change
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX
. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false
&mvn clean test-compile failsafe:integration-test
to make sure unit-test and integration-test pass.