logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: Memory leak in TCP appender in odd case
Date Fri, 02 Sep 2016 06:40:41 GMT
I just had a look at the code but I can't find a place where there's any attempt to get the
bytes out of the ByteArrayOutputStream. 
If we're going to drop the messages anyway we should use a NullOutputStream. 

Sent from my iPhone

> On 2016/09/02, at 12:46, Gary Gregory <garydgregory@gmail.com> wrote:
> 
> Yeah, but if the server is never up, the array will keep on growing...
> 
> Gary
> 
>> On Wed, Aug 31, 2016 at 4:47 PM, Remko Popma <remko.popma@gmail.com> wrote:
>> (Without looking at the code) I vaguely remember that class has a reconnect mechanism.
Is the ByteArrayOutputStream used during the reconnect?
>> 
>> Sent from my iPhone
>> 
>>> On 2016/09/01, at 7:16, Gary Gregory <garydgregory@gmail.com> wrote:
>>> 
>>> We have this code in org.apache.logging.log4j.core.net.TcpSocketManager.TcpSocketManagerFactory.createManager(String,
FactoryData):
>>> 
>>>         @Override
>>>         public TcpSocketManager createManager(final String name, final FactoryData
data) {
>>> 
>>>             InetAddress inetAddress;
>>>             OutputStream os;
>>>             try {
>>>                 inetAddress = InetAddress.getByName(data.host);
>>>             } catch (final UnknownHostException ex) {
>>>                 LOGGER.error("Could not find address of " + data.host, ex, ex);
>>>                 return null;
>>>             }
>>>             try {
>>>                 // LOG4J2-1042
>>>                 final Socket socket = new Socket();
>>>                 socket.connect(new InetSocketAddress(data.host, data.port), data.connectTimeoutMillis);
>>>                 os = socket.getOutputStream();
>>>                 return new TcpSocketManager(name, os, socket, inetAddress, data.host,
data.port,
>>>                         data.connectTimeoutMillis, data.delayMillis, data.immediateFail,
data.layout);
>>>             } catch (final IOException ex) {
>>>                 LOGGER.error("TcpSocketManager (" + name + ") " + ex, ex);
>>>                 os = new ByteArrayOutputStream();
>>>             }
>>>             if (data.delayMillis == 0) {
>>>                 return null;
>>>             }
>>>             return new TcpSocketManager(name, os, null, inetAddress, data.host,
data.port, data.connectTimeoutMillis,
>>>                     data.delayMillis, data.immediateFail, data.layout);
>>>         }
>>> 
>>> Notice:
>>> 
>>>                 LOGGER.error("TcpSocketManager (" + name + ") " + ex, ex);
>>>                 os = new ByteArrayOutputStream();
>>> 
>>> What is the point of that? It creates an unbound stream in RAM, creating one
problem while dealing with another.
>>> 
>>> I think it would be better to not create the manager at all.
>>> 
>>> Thoughts?
>>> 
>>> Gary
>>> 
>>> -- 
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
>>> Java Persistence with Hibernate, Second Edition
>>> JUnit in Action, Second Edition
>>> Spring Batch in Action
>>> Blog: http://garygregory.wordpress.com 
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

Mime
View raw message