John, Looks good for me.
-Dmitry On 2013-05-31 21:06, John Zavgren wrote: > All: > I have a revised webrev image that's ready for review: > http://cr.openjdk.java.net/~jzavgren/8008972/webrev.04/ > > Thanks > John > > On 05/13/2013 04:14 PM, John Zavgren wrote: >> Greetings: >> I posted a new webrev image: >> http://cr.openjdk.java.net/~jzavgren/8008972/webrev.03/ >> that fixes a memory leak. >> >> The leak in >> src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c is >> caused by an unfortunate interplay between two actions: >> 1.) dynamically allocating a larger packet buffer when the length of >> an incoming packet exceeds MAX_BUFFER_LEN. >> 2.) truncating a received packet so that it's length equals >> MAX_PACKET_LEN, when it exceeds this value. >> >> Action number two was embedded inside the code that implemented action >> number one, and action number two can change the length of the packet. >> Unfortunately, the original packet length was used as an indication >> that the packet buffer needed to be freed, and because the length of >> the packet may have changed after the allocation, the free() statement >> wasn't being executed in some cases. >> >> I fixed the problem by moving action number two so that it always >> precedes action number 1. This ensures that the packet length retains >> the same value across the malloc() and the free() operations, and >> therefore packet length can now be used as a reliable indication that >> dynamically allocated memory must be freed. >> >> I also noticed that there is a procedure in >> src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c that >> also dynamically allocates memory in a similar way, but it doesn't >> implement action number two. I added it. >> >> I also took advantage of this opportunity to fix spelling errors in >> other files in the same directory as the two previously mentioned files. >> >> >> On 03/04/2013 01:17 PM, Chris Hegarty wrote: >>> 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 >>>> >> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
