Re: Datagram socket leak

2011-09-09 Thread Alan Bateman
Michael McMahon wrote: http://cr.openjdk.java.net/~michaelm/7088747/webrev.1/ There are no changes in the first two files, only Socket.java Looks much better. I assume that the check if address is null can be removed at L423. -Alan

Re: Datagram socket leak

2011-09-09 Thread Michael McMahon
http://cr.openjdk.java.net/~michaelm/7088747/webrev.1/ There are no changes in the first two files, only Socket.java - Michael. On 09/09/11 14:08, Alan Bateman wrote: Michael McMahon wrote: Yes, the regression tests picked that up as well. Well spotted. http://cr.openjdk.java.net/~michaelm/7

Re: Datagram socket leak

2011-09-09 Thread Michael McMahon
Alan, It didn't occur to me to use that new multi-catch construct. I've actually just committed the previous version. But, I think this is clearer so I'd like to make this change under a new CR. - Michael. On 09/09/11 14:08, Alan Bateman wrote: Michael McMahon wrote: Yes, the regression test

Re: Datagram socket leak

2011-09-09 Thread Alan Bateman
Michael McMahon wrote: Yes, the regression tests picked that up as well. Well spotted. http://cr.openjdk.java.net/~michaelm/7085981/webrev.5/ DatagramSocket looks okay to me. An alternative might be: try { : } catch (SocketException | IllegalArgumentException | SecurityException e) { clo

Re: Datagram socket leak

2011-09-09 Thread Michael McMahon
On 08/09/11 21:40, Alan Bateman wrote: Michael McMahon wrote: Sigh. Hopefully this is the last webrev. http://cr.openjdk.java.net/~michaelm/7085981/webrev.4/ Socket.java changed from last one. If there's no objection I'll push this version. - Michael. DatagramSocket(SocketAddress) creates an

Re: Datagram socket leak

2011-09-08 Thread Alan Bateman
Michael McMahon wrote: Sigh. Hopefully this is the last webrev. http://cr.openjdk.java.net/~michaelm/7085981/webrev.4/ Socket.java changed from last one. If there's no objection I'll push this version. - Michael. DatagramSocket(SocketAddress) creates an unbound socket if the parameter is null

Re: Datagram socket leak

2011-09-08 Thread Chris Hegarty
t;< if (localAddr != null) bind(localAddr); if (address != null) connect(address); } catch (IOException e) { close(); throw e; } } -Original Message- From: Michael McMahon [mailto:michael.x.mcma...@oracle.com] Sent: Thursday, September 08, 2011 4:11 AM To: chris hegarty Cc: Salter, Th

Re: Datagram socket leak

2011-09-08 Thread Michael McMahon
Thursday, September 08, 2011 4:11 AM To: chris hegarty Cc: Salter, Thomas A; net-dev@openjdk.java.net Subject: Re: Datagram socket leak On 07/09/11 21:54, chris hegarty wrote: On 07/09/2011 17:24, Michael McMahon wrote: Hi all, I've posted a webrev for this at: http://cr.openjdk.java.net/

RE: Datagram socket leak

2011-09-08 Thread Salter, Thomas A
el McMahon [mailto:michael.x.mcma...@oracle.com] Sent: Thursday, September 08, 2011 4:11 AM To: chris hegarty Cc: Salter, Thomas A; net-dev@openjdk.java.net Subject: Re: Datagram socket leak On 07/09/11 21:54, chris hegarty wrote: > On 07/09/2011 17:24, Michael McMahon wrote: >> Hi all,

Re: Datagram socket leak

2011-09-08 Thread Chris Hegarty
On 09/ 8/11 12:12 PM, Michael McMahon wrote: . http://cr.openjdk.java.net/~michaelm/7085981/webrev.3/ The changes look good to me. -Chris. Thanks, Michael.

Re: Datagram socket leak

2011-09-08 Thread Michael McMahon
On 07/09/11 21:54, chris hegarty wrote: On 07/09/2011 17:24, Michael McMahon wrote: Hi all, I've posted a webrev for this at: http://cr.openjdk.java.net/~michaelm/7085981/webrev.1/ After checking Socket and ServerSocket, I believe they are not actually affected by this issue. The IllegalArgum

Re: Datagram socket leak

2011-09-08 Thread Michael McMahon
On 07/09/11 21:54, chris hegarty wrote: On 07/09/2011 17:24, Michael McMahon wrote: Hi all, I've posted a webrev for this at: http://cr.openjdk.java.net/~michaelm/7085981/webrev.1/ After checking Socket and ServerSocket, I believe they are not actually affected by this issue. The IllegalArgum

Re: Datagram socket leak

2011-09-07 Thread chris hegarty
On 07/09/2011 17:24, Michael McMahon wrote: Hi all, I've posted a webrev for this at: http://cr.openjdk.java.net/~michaelm/7085981/webrev.1/ After checking Socket and ServerSocket, I believe they are not actually affected by this issue. The IllegalArgumentExceptions are thrown before the imp

Re: Datagram socket leak

2011-09-07 Thread Michael McMahon
for this bug as well. Michael. -Original Message- From: Michael McMahon [mailto:michael.x.mcma...@oracle.com] Sent: Thursday, September 01, 2011 12:47 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subject: Re: Datagram socket leak Thomas, Thanks for pointing this out. We overlooked this in

Re: Datagram socket leak

2011-09-01 Thread Michael McMahon
--- From: Michael McMahon [mailto:michael.x.mcma...@oracle.com] Sent: Thursday, September 01, 2011 12:47 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subject: Re: Datagram socket leak Thomas, Thanks for pointing this out. We overlooked this in the recent change in this area. One thing though

RE: Datagram socket leak

2011-09-01 Thread Salter, Thomas A
c: net-dev@openjdk.java.net Subject: Re: Datagram socket leak Thomas, Thanks for pointing this out. We overlooked this in the recent change in this area. One thing though, in the second change to DatagramSocket we can't just check for isBound() since the socket might legitimately be unbound (bindaddr

Re: Datagram socket leak

2011-09-01 Thread Michael McMahon
if( !isBound() ) <close(); <} -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Monday, August 29, 2011 2:33 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subject: Re: Datagram socket leak Ah ok. I finally get it.

RE: Datagram socket leak

2011-08-29 Thread Salter, Thomas A
try { 167,170d165 < } finally { < if( !isBound() ) < close(); < } -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Monday, August 29, 2011 2:33 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subjec

RE: Datagram socket leak

2011-08-29 Thread Salter, Thomas A
ows. Tom -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Monday, August 29, 2011 2:33 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subject: Re: Datagram socket leak Ah ok. I finally get it. In which case I think you original changes should be f

Re: Datagram socket leak

2011-08-29 Thread Chris Hegarty
try { getImpl().bind(port, iaddr); } catch (SocketException e) { getImpl().close(); throw e; } bound = true; } Tom. -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Monday, August 29, 2011 1:56 PM

RE: Datagram socket leak

2011-08-29 Thread Salter, Thomas A
se(); throw e; } bound = true; } Tom. -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Monday, August 29, 2011 1:56 PM To: Salter, Thomas A Cc: net-dev@openjdk.java.net Subject: Re: Datagram socket leak [take two!] Tom, Thi

Re: Datagram socket leak

2011-08-29 Thread Chris Hegarty
[take two!] Tom, This specific area of code was changed recently due to CR 7035556 [1], changeset [2], and this issue was discussed during the code review [3]. Essentially, bind() already closes the impl internally before throwing the exception. Does this resolve the issue for you? Or do you

Re: Datagram socket leak

2011-08-29 Thread Chris Hegarty
I haven't looked at the code in question in detail, but from the code snippet, won't the finally close the socket ( since it is not isBound )? -Chris. On 08/29/11 03:27 PM, Salter, Thomas A wrote: There appears to be a socket leak in both DatagramSocket and MulticastSocket constructors. Both c

Datagram socket leak

2011-08-29 Thread Salter, Thomas A
There appears to be a socket leak in both DatagramSocket and MulticastSocket constructors. Both classes have constructors that create a socket and then attempt to bind. The bind can fail with a variety of exceptions none of which are caught by the constructor. Thus, the actual system socket t