On Sun, Aug 28, 2016 at 4:01 PM, Remko Popma <[email protected]> wrote:
> I don't think the close() method should be renamed to closeOutputStream(), > at least for > MemoryMappedFileManager, > RandomAccessFileManager, and > RollingRandomAccessFileManager. > > The above three have a dummy OutputStream because they inherit from > OuputStreamManager, but closing this stream does not do much. Naming the > method such would be emphasizing the wrong thing. > Hi Remko, You could argue it the other way around, it is the fact that these appender extend OuputStreamManager without really using an output stream that is odd and/or misleading. This seems a bigger oddity than the name of a method, > > Other Managers may manage yet other resources. > What is wrong with a generic close() method? > There is a close method but it is a method that is really implemented by the releaseSub() method. > > Also, you state AbstractManager should implement AutoCloseable > without any indication why. There must be some reason why you think so. > Please share it. > I updated the ticket and the Javadoc. Thank you, Gary > > > > On Mon, Aug 29, 2016 at 5:22 AM, <[email protected]> wrote: > >> Repository: logging-log4j2 >> Updated Branches: >> refs/heads/LOG4J2-1553 [created] 13c3c2479 >> >> >> [LOG4J2-1553] AbstractManager should implement AutoCloseable. Rename >> AbstractManager.close() to clone{Resource}(). >> >> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit >> /b73c589c >> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/b73c589c >> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/b73c589c >> >> Branch: refs/heads/LOG4J2-1553 >> Commit: b73c589c72bd526846ce045c10e5146212a96c88 >> Parents: f936a6d >> Author: Gary Gregory <[email protected]> >> Authored: Sun Aug 28 12:04:55 2016 -0700 >> Committer: Gary Gregory <[email protected]> >> Committed: Sun Aug 28 12:04:55 2016 -0700 >> >> ---------------------------------------------------------------------- >> .../logging/log4j/core/appender/MemoryMappedFileManager.java | 2 +- >> .../apache/logging/log4j/core/appender/OutputStreamManager.java | 4 >> ++-- >> .../logging/log4j/core/appender/RandomAccessFileManager.java | 2 +- >> .../org/apache/logging/log4j/core/appender/WriterManager.java | 4 >> ++-- >> .../logging/log4j/core/appender/rolling/RollingFileManager.java | 2 +- >> .../core/appender/rolling/RollingRandomAccessFileManager.java | 2 +- >> .../java/org/apache/logging/log4j/core/net/TcpSocketManager.java | 4 >> ++-- >> 7 files changed, 10 insertions(+), 10 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/MemoryMappedFileManager.java >> ---------------------------------------------------------------------- >> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/MemoryMappedFileManager.java b/log4j-core/src/main/java/org >> /apache/logging/log4j/core/appender/MemoryMappedFileManager.java >> index e238258..a856610 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/MemoryMappedFileManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/MemoryMappedFileManager.java >> @@ -156,7 +156,7 @@ public class MemoryMappedFileManager extends >> OutputStreamManager { >> } >> >> @Override >> - public synchronized void close() { >> + public synchronized void closeOutputStream() { >> final long position = mappedBuffer.position(); >> final long length = mappingOffset + position; >> try { >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/OutputStreamManager.java >> ---------------------------------------------------------------------- >> diff --git >> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java >> b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/OutputStreamManager.java >> index 698409a..97a9dc6 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/OutputStreamManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/OutputStreamManager.java >> @@ -126,7 +126,7 @@ public class OutputStreamManager extends >> AbstractManager implements ByteBufferDe >> @Override >> public void releaseSub() { >> writeFooter(); >> - close(); >> + closeOutputStream(); >> } >> >> /** >> @@ -287,7 +287,7 @@ public class OutputStreamManager extends >> AbstractManager implements ByteBufferDe >> flushDestination(); >> } >> >> - protected synchronized void close() { >> + protected synchronized void closeOutputStream() { >> flush(); >> final OutputStream stream = os; // access volatile field only >> once per method >> if (stream == null || stream == System.out || stream == >> System.err) { >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/RandomAccessFileManager.java >> ---------------------------------------------------------------------- >> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/RandomAccessFileManager.java b/log4j-core/src/main/java/org >> /apache/logging/log4j/core/appender/RandomAccessFileManager.java >> index 7ab0fe3..6aca266 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/RandomAccessFileManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/RandomAccessFileManager.java >> @@ -98,7 +98,7 @@ public class RandomAccessFileManager extends >> OutputStreamManager { >> } >> >> @Override >> - public synchronized void close() { >> + public synchronized void closeOutputStream() { >> flush(); >> try { >> randomAccessFile.close(); >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/WriterManager.java >> ---------------------------------------------------------------------- >> diff --git >> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/WriterManager.java >> b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/WriterManager.java >> index b3b9e73..cee99e8 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/WriterManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/WriterManager.java >> @@ -61,7 +61,7 @@ public class WriterManager extends AbstractManager { >> } >> } >> >> - protected synchronized void close() { >> + protected synchronized void closeWriter() { >> final Writer w = writer; // access volatile field only once per >> method >> try { >> w.close(); >> @@ -100,7 +100,7 @@ public class WriterManager extends AbstractManager { >> @Override >> public void releaseSub() { >> writeFooter(); >> - close(); >> + closeWriter(); >> } >> >> protected void setWriter(final Writer writer) { >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/rolling/RollingFileManager.java >> ---------------------------------------------------------------------- >> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingFileManager.java b/log4j-core/src/main/java/org >> /apache/logging/log4j/core/appender/rolling/RollingFileManager.java >> index 3f8ba2a..e00319d 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingFileManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingFileManager.java >> @@ -242,7 +242,7 @@ public class RollingFileManager extends FileManager { >> final RolloverDescription descriptor = >> strategy.rollover(this); >> if (descriptor != null) { >> writeFooter(); >> - close(); >> + closeOutputStream(); >> if (descriptor.getSynchronous() != null) { >> LOGGER.debug("RollingFileManager executing >> synchronous {}", descriptor.getSynchronous()); >> try { >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/appender/rolling/RollingRandomAccessFileManager.java >> ---------------------------------------------------------------------- >> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingRandomAccessFileManager.java >> b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingRandomAccessFileManager.java >> index 8b39209..f27d445 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingRandomAccessFileManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/app >> ender/rolling/RollingRandomAccessFileManager.java >> @@ -128,7 +128,7 @@ public class RollingRandomAccessFileManager extends >> RollingFileManager { >> } >> >> @Override >> - public synchronized void close() { >> + public synchronized void closeOutputStream() { >> flush(); >> try { >> randomAccessFile.close(); >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/b >> 73c589c/log4j-core/src/main/java/org/apache/logging/log4j/co >> re/net/TcpSocketManager.java >> ---------------------------------------------------------------------- >> diff --git >> a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/TcpSocketManager.java >> b/log4j-core/src/main/java/org/apache/logging/log4j/core/net >> /TcpSocketManager.java >> index 4d83d97..a34ce30 100644 >> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net >> /TcpSocketManager.java >> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net >> /TcpSocketManager.java >> @@ -147,8 +147,8 @@ public class TcpSocketManager extends >> AbstractSocketManager { >> } >> >> @Override >> - protected synchronized void close() { >> - super.close(); >> + protected synchronized void closeOutputStream() { >> + super.closeOutputStream(); >> if (connector != null) { >> connector.shutdown(); >> connector.interrupt(); >> >> > -- E-Mail: [email protected] | [email protected] Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
