RFR JDK-8008972

[Chris Hegarty]

OK, John I get your point, but given the definitions of these values 
(below) this "potential" leak can never happen. It is unlikely that 
MAX_BUFFER_LEN will ever exceed MAX_PACKET_LEN ;-)

Shared:
  #define MAX_PACKET_LEN 65536

Unix:
  #ifdef _LP64
  #define MAX_BUFFER_LEN 65536
  #define MAX_HEAP_BUFFER_LEN 131072
  #else
  #define MAX_BUFFER_LEN 8192
  #define MAX_HEAP_BUFFER_LEN 65536
  #endif

Windows:
  #define MAX_BUFFER_LEN          2048
  #define MAX_HEAP_BUFFER_LEN     65536

-Chris.

On 03/04/2013 06:35 PM, John Zavgren wrote:
> All:
>
> I understand that it may be hard to find the memory leak problem I'm trying to solve... let me try to explain.
>
> In the file:
> src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
>
> there is a conditional memory allocation:
>      if (packetBufferLen > MAX_BUFFER_LEN) {
>          /* Note: the buffer needn't be greater than 65,536 (0xFFFF)
>           * the max size of an IP packet. Anything bigger is truncated anyway.
>           */
>          if (packetBufferLen > MAX_PACKET_LEN) {
>              packetBufferLen = MAX_PACKET_LEN;
>          }
>          fullPacket = (char *)malloc(packetBufferLen);
>          memoryAllocated = TRUE;
>          if (!fullPacket) {
>              JNU_ThrowOutOfMemoryError(env, "Native heap allocation failed");
>              return -1;
>          }
>      } else {
>          fullPacket = &(BUF[0]);
>      }
> Notice that the condition for allocating the memory: if (packetBufferLen > MAX_BUFFER_LEN) may not remain a valid condition for freeing it because the value of packetBufferLen may be changed by the statement:
>          if (packetBufferLen > MAX_PACKET_LEN) {
>              packetBufferLen = MAX_PACKET_LEN;
>          }
> A few lines later, there is a conditional free call:
>   if (packetBufferLen > MAX_BUFFER_LEN) {
> ...
>
> Basically, the logic goes like this.. if condition "A" is true allocate memory, and then later if condition "A prime" is true free it. The conditions "A" and "A prime" are frequently the same, but sometimes they are not.
>
> A similar potential memory leak occurs in the "companion" file:
> src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c
>
> I hope this helps.
>
> John
>
> ----- Original Message -----
> From: chris.hegarty at oracle.com
> To: net-dev at openjdk.java.net, john.zavgren at oracle.com
> Sent: Monday, March 4, 2013 1:17:31 PM GMT -05:00 US/Canada Eastern
> Subject: Re: RFR JDK-8008972
>
> On 03/04/2013 04:28 PM, John Zavgren wrote:
>> Greetings:
>>
>> I've posted a webrev image of a memory leak fix:
>> http://cr.openjdk.java.net/~jzavgren/8008972/webrev.01/
>
> Sorry John, I don't see what the problem is that you are trying to
> solve? Can you please explain why the original code has problems?
>
> Also, note that there are other places in the same function that do the
> very same check.
>
> -Chris.
>
>>
>> Thanks!
>> John
>>