[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[
https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17702001#comment-17702001
]
Gary D. Gregory commented on COMPRESS-502:
--
Removing writing to std err is a no-brainer IMO, so I'll at least do that now.
WRT removing
{*}org.apache.commons.compress.archivers.zip.ZipFile.finalize(){*}, this would
break out the current Javadoc contract to provide a replacement for
*java.util.zip.ZipFile* which implements *finalize()* to call {*}close(){*}, so
we should not remove it for now.
> Allow to disable closing files in the finalizer of ZipFile
> --
>
> Key: COMPRESS-502
> URL: https://issues.apache.org/jira/browse/COMPRESS-502
> Project: Commons Compress
> Issue Type: Improvement
> Components: Compressors
>Affects Versions: 1.19
>Reporter: Dominik Stadler
>Priority: Major
>
> Apache POI uses commons-compress for handling ZipFiles. We found that it
> sometimes does some auto-close magic in the finalizer of the ZipFile class
> with printing out to stderr, see
> https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652.
>
> This has some shortcomings:
> * It prints to stderr which many large-scale applications try to avoid by
> using some logging framework, thus this output might "vanish" unseen in some
> installations or might cause unexpected side-effects
> * It prevents us from using tools for checking file leaks, e.g. we use
> [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing
> test-runs for leaked file-handles, but commons-compress prevents this because
> it "hides" the unclosed file from this functionality
> * The behavior of automatic closing and reporting the problem is
> non-reproducible because it depends on finalizer/garbage-collection and thus
> any re-runs or unit-tests usually do not show the same behavior
>
> There are some fairly simple options to improve this:
> * Allow to disable this functionality via configuration/system-property/...
> * Make this "pluggable" so a logging-framework can be plugged-in or closing
> can be prevented for certain runs
>
> I can provide a simple patch if you state which approach you think would make
> most sense here.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17028444#comment-17028444 ] Stefan Bodewig commented on COMPRESS-502: - Sounds as if both [~ggregory] and I could live with either a constructor-arg or an instance setter. > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023873#comment-17023873 ] Gary D. Gregory commented on COMPRESS-502: -- How is a system property any different from a global variable? It is not :( The only kind of toggle that would allow for multi-tenancy for this library would be an instance variable. > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023864#comment-17023864 ] Dominik Stadler commented on COMPRESS-502: -- What are acceptable options then? A system property? A setter on the object? > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023795#comment-17023795 ] Gary D. Gregory commented on COMPRESS-502: -- -1 to using a global variable for the usual and obvious reasons > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[
https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023640#comment-17023640
]
Dominik Stadler commented on COMPRESS-502:
--
We would like to use another tool to better report the actual location of where
the file was opened as the single output to stderr can be too less to find the
location of allocation in a fairly sized application.
So I mostly would like to not have "close()" be run as part of the finalizer.
The output to stderr is another shortcoming which might be unwanted for others.
A simple solution as far as I see would be to make it pluggable via a simple
static setter and use the current System.err.println as default.
E.g. something like the following (did not compile this, so might not fully
work this way):
private static Function finalizeFun = (file) -> {
if (!closed) {
System.err.println("Cleaning up unclosed ZipFile for archive " +
archiveName);
file.close();
}
};
public static void setFinalizer(Function fun) {
finalizeFun = fun;
}
protected void finalize() throws Throwable {
try {
finalizeRunnable->apply(this);
} finally {
super.finalize();
}
}
> Allow to disable closing files in the finalizer of ZipFile
> --
>
> Key: COMPRESS-502
> URL: https://issues.apache.org/jira/browse/COMPRESS-502
> Project: Commons Compress
> Issue Type: Improvement
> Components: Compressors
>Affects Versions: 1.19
>Reporter: Dominik Stadler
>Priority: Major
>
> Apache POI uses commons-compress for handling ZipFiles. We found that it
> sometimes does some auto-close magic in the finalizer of the ZipFile class
> with printing out to stderr, see
> https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652.
>
> This has some shortcomings:
> * It prints to stderr which many large-scale applications try to avoid by
> using some logging framework, thus this output might "vanish" unseen in some
> installations or might cause unexpected side-effects
> * It prevents us from using tools for checking file leaks, e.g. we use
> [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing
> test-runs for leaked file-handles, but commons-compress prevents this because
> it "hides" the unclosed file from this functionality
> * The behavior of automatic closing and reporting the problem is
> non-reproducible because it depends on finalizer/garbage-collection and thus
> any re-runs or unit-tests usually do not show the same behavior
>
> There are some fairly simple options to improve this:
> * Allow to disable this functionality via configuration/system-property/...
> * Make this "pluggable" so a logging-framework can be plugged-in or closing
> can be prevented for certain runs
>
> I can provide a simple patch if you state which approach you think would make
> most sense here.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023583#comment-17023583 ] Stefan Bodewig commented on COMPRESS-502: - Hmm, if the ZipFile instance is properly closed then there is no message at all. If people want to disable the feature because they know they always close the file then they wouldn't need to disable it because the message never appears :) > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17023582#comment-17023582 ] Stefan Bodewig commented on COMPRESS-502: - I'm afraid the finalizer has there for a very long time and we can't simply remove it. Plugging in a logging framework would address the message going to System.err but not address the two other shortcomings you mention. In the case of Tika you probably want to make sure to never run the finalizer which probably rules out a system property. This would leave us with yet another constructor argument. > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (COMPRESS-502) Allow to disable closing files in the finalizer of ZipFile
[ https://issues.apache.org/jira/browse/COMPRESS-502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17021420#comment-17021420 ] Dominik Stadler commented on COMPRESS-502: -- Issue COMPRESS-473 handles the fact that overriding finalize() is discouraged in general anyway. > Allow to disable closing files in the finalizer of ZipFile > -- > > Key: COMPRESS-502 > URL: https://issues.apache.org/jira/browse/COMPRESS-502 > Project: Commons Compress > Issue Type: Improvement > Components: Compressors >Affects Versions: 1.19 >Reporter: Dominik Stadler >Priority: Major > > Apache POI uses commons-compress for handling ZipFiles. We found that it > sometimes does some auto-close magic in the finalizer of the ZipFile class > with printing out to stderr, see > https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=blob;f=src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java;h=23194560dace91d8052626f3bdc8f765f9c46d7e;hb=HEAD#l652. > > This has some shortcomings: > * It prints to stderr which many large-scale applications try to avoid by > using some logging framework, thus this output might "vanish" unseen in some > installations or might cause unexpected side-effects > * It prevents us from using tools for checking file leaks, e.g. we use > [https://github.com/kohsuke/file-leak-detector/] heavily for analyzing > test-runs for leaked file-handles, but commons-compress prevents this because > it "hides" the unclosed file from this functionality > * The behavior of automatic closing and reporting the problem is > non-reproducible because it depends on finalizer/garbage-collection and thus > any re-runs or unit-tests usually do not show the same behavior > > There are some fairly simple options to improve this: > * Allow to disable this functionality via configuration/system-property/... > * Make this "pluggable" so a logging-framework can be plugged-in or closing > can be prevented for certain runs > > I can provide a simple patch if you state which approach you think would make > most sense here. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
