unfetteredwind commented on pull request #2369:
URL: https://github.com/apache/thrift/pull/2369#issuecomment-823735239
OK, first of all, let me briefly explain why this judgment condition is
written.
There are two writing conditions:
1、The value of allocate is true.
2、The type is not TYPE_STRING type in the base type.
1、In this document, the function generate_deserialize_field is invoked in
the following four locations:
struct:generate_struct_reader allocate：default value:
false 3579 3664
map: generate_deserialize_map_element allocate：default value: true
4271 4294
set: generate_deserialize_set_element allocate：default value: true
4305 4316
list: generate_deserialize_list_element allocate：default value: true
4325 4338
Check the function invoked by each. If the value of allocate is false, new
is not triggered(declare_local_variable is not invoked in the struct ). In this
case, free is not required.
When the value of allocate is true, the declare_local_variable function is
invoked in the map, set, and list to trigger new. In this case, you need to
perform the free operation when an exception occurs.
So that's one of my judgements.
2、And going back to this function : generate_deserialize_list_element.
The judgment condition is in the base_type branch. 4020
As you can see, if the type is TYPE_STRING, there is no leakage problem in
this part,。Because he did special treatment., 4023-4028.
For the remaining types, if they enter the exception branch after new,
memory leakage will be triggered.
That's another criterion for me. If the condition is met, it needs to be
processed.
That's why I chose the condition. And, perhaps the judgment is not rigorous
enough, and my intention is to raise this issue so that people can see this
issue and push this issue to be resolved.
As for the test..., it's working on it.And....I'm lazy. Excuse me.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
