[Bug 2955] Resource leak in Thermostat byteman helper

[bugzilla-daemon at icedtea.classpath.org]

http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2955

Severin Gehwolf <sgehwolf at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at icedtea.classpat |ebaron at redhat.com
                   |h.org                       |

--- Comment #10 from Severin Gehwolf <sgehwolf at redhat.com> ---
(In reply to Elliott Baron from comment #8)
> (In reply to Severin Gehwolf from comment #7)
> > (In reply to Elliott Baron from comment #1)
> > > Alternatively, we could use the approach you mentioned of using a separate
> > > connection for each send operation. I don't believe any additional API is
> > > needed to make this work. I think the following should be sufficient:
> > > > ClientIPCService ipcService = ClientIPCServiceFactory.getIPCService(ipcConfig);
> > > > ByteChannel channel = ipcService.connectToServer(socketName);
> > > > channel.write(...);
> > > > channel.close();
> > 
> > I believe patch in comment 6 would do this, but it does not fix the resource
> > leak. Thoughts?
> 
> After investigating, I believe I've figured out the problem. A new instance
> of ThermostatHelper is not only created for each send operation, it is
> created for every time sleep is called.

That makes sense. The rule instruments sleep(), the trigger point.

> The Byteman rule we're using invokes
> send for every 10 times that sleep is called. This means only 1 out of 10
> sockets is closed if we try to close only when send is called. Another small
> issue with the patch in comment 6, is that there is an early return
> statement in the for-loop that prevents close from being called unless an
> exception occurs.

Right, thanks. I totally missed the early return. Ugh :-/

> Here is another patch that appears to fix the resource leak. The idea is to
> create the transport in the send method instead of the constructor. I had to
> add some synchronization to the close method, but if we intend to have a
> separate connection per send operation, we could remove most (or all) of the
> synchronization in Transport.

I can confirm that this fixes the issue too. I'll do some more testing and will
report back to you.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20160513/47e4dc29/attachment.html>