[ 
https://issues.apache.org/jira/browse/THRIFT-5322?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17248546#comment-17248546
 ] 

Jens Geyer edited comment on THRIFT-5322 at 12/13/20, 10:03 AM:
----------------------------------------------------------------

{quote}
But I do not think implementing TConfiguration in go is required to fix this 
issue
{quote}
You are absolutely right, its not needed to fix the ssue at hand. 

My primary goal was to make you aware of a) what we already have implemented to 
prevent people from doing double work and b) that the problem might be slightly 
bigger than the actual issue suggests. Because we also have lists/sets/maps 
where memory effectively is allocated based on the number of elements which 
also imply a certain minimum amount of data that has to follow - same as with 
binary/strings.

Last not least, TConfiguration is merely an attempt to consolidate and 
(wherevere necessary) standardize the defaults of all of those limits we have 
accumulated during the years and make them configurable in a standardized way 
for all target languages. So technically it does not solve a thing w/regard to 
this ticket at all. Nevertheless it seems a good thing to have for the future.


was (Author: jensg):
{quote}
But I do not think implementing TConfiguration in go is required to fix this 
issue
{quote}
You are absolutely right, its not needed to fix the ssue at hand. 

My primary goal was to make you aware of a) what we already have implemented to 
prevent people from doing double work and b) that the problem might be slightly 
bigger than the actual issue suggests. Because we also have lists/sets/maps 
where memory effectively is allocated based on the number of elements which 
also imply a certain minimum amount of data that has to follow - same as with 
binary/strings.

> Go compact_protocol allocating unreasonable buffer size
> -------------------------------------------------------
>
>                 Key: THRIFT-5322
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5322
>             Project: Thrift
>          Issue Type: Bug
>          Components: Go - Library
>    Affects Versions: 0.13.0
>            Reporter: Juraci Paixão Kröhling
>            Assignee: Yuxuan Wang
>            Priority: Major
>         Attachments: main.go
>
>          Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> I don't yet know all the pieces to this puzzle, and it's quite possible that 
> the problem is on our side, but we use the Thrift Go library in the Jaeger 
> Agent and we are seeing a case where the memory consumption for a payload of 
> 4k bytes to result in a buffer allocation in the compact_protocol.go with 
> unreasonable sizes. I found buffers of 1.4GiB while debugging the issue.
>  
> This is the code that we are seeing this memory usage:
> [https://github.com/apache/thrift/blob/b75e88a33d67ae05ef9b5fa001d2a63a2effe377/lib/go/thrift/compact_protocol.go#L556-L577]
>  
> Here's more information about this, including a reproducer and initial 
> diagnostics:
> [https://github.com/jaegertracing/jaeger/issues/2638#issuecomment-741848201]
>  
> As mentioned above, I'm still getting all the pieces together, but perhaps 
> you've seen this before or know what might be going on. What I know for sure 
> at the moment is that this happens on malformed payloads, but I would expect 
> the library to have an upper limit on the buffer size.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to