You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by "Kay Kay (JIRA)" <ji...@apache.org> on 2008/12/14 18:39:44 UTC

[jira] Created: (SOLR-914) Presence of finalize() in the codebase

Presence of finalize() in the codebase 
---------------------------------------

                 Key: SOLR-914
                 URL: https://issues.apache.org/jira/browse/SOLR-914
             Project: Solr
          Issue Type: Improvement
          Components: clients - java
    Affects Versions: 1.3
         Environment: Tomcat 6, JRE 6
            Reporter: Kay Kay
             Fix For: 1.4


There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 

$ find . -name *.java | xargs grep finalize
./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {

May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kay Kay updated SOLR-914:
-------------------------

    Component/s:     (was: Analysis)
                 clients - java

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656879#action_12656879 ] 

Noble Paul commented on SOLR-914:
---------------------------------

bq.we are noticing instances where the gc thread postpones collecting the objects until finalize() is invoked. 

It is true but GC thread only has to wait for the time duration of the finalize() method call. So which class exactly is the culprit here in your case?


> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: Analysis
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hoss Man updated SOLR-914:
--------------------------

    Attachment: SOLR-914.patch

revised patch with the improvements i mentioned earlier, also fixes a cut/paste mistake in one of the log messages.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Hoss Man
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch, SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12730273#action_12730273 ] 

Noble Paul commented on SOLR-914:
---------------------------------

bq.Code to release resources should be avoided as a finalize is no equivalent to a C++ dtor.

yes. But if the user has forgotten to do so  It is not a good idea to punish him by blowing up. A warning should be enough. 


> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Noble Paul
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Noble Paul updated SOLR-914:
----------------------------

    Attachment: SOLR-914.patch

all the finalize () methods check once if the object is already closed. if not log a warning and close

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Noble Paul
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12725930#action_12725930 ] 

Noble Paul commented on SOLR-914:
---------------------------------

bq.Logging a warning seems like the right way to go to me

so do you mean , logging a warning and do not do cleanup. or log a warning and do the cleanup ?

I would prefer the latter because the system will continue to work 

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Lance Norskog (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660668#action_12660668 ] 

Lance Norskog commented on SOLR-914:
------------------------------------

A note: it is a good practice to use finalize() methods to check that a resource has already been released. It should log an error if the resource has not been released. Finalize() methods are all parked on one queue and that queue can back up. This can eventually cause the app to lock up. This is why it is not good to do I/O actions (like close a database connection) inside the finalize method.

If the method only checks an internal marker, that will not cause a backup.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656421#action_12656421 ] 

Noble Paul commented on SOLR-914:
---------------------------------

The fact that there are only 5 instances of these tells that it may not be an put inadvertently.

two of them are my own creation. The code does not rely on these methods . But the consumers of these classes may forget to call the close/destroy methods explicitly . In such cases finalize() is just a fallback option

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12745790#action_12745790 ] 

Noble Paul commented on SOLR-914:
---------------------------------

I guess this should be enough.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Hoss Man
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch, SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Noble Paul updated SOLR-914:
----------------------------

    Assignee:     (was: Noble Paul)

There seems to be no consensus on the fix. Unassigning myself

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656424#action_12656424 ] 

Kay Kay commented on SOLR-914:
------------------------------

Precisely. I believe - we should get away with the same since in our case  - when we try to shutdown and restart SolrCore - we are noticing instances where the gc thread postpones collecting the objects until finalize() is invoked. 

Given that finalize() is spec-d such that there is no guarantee of being called  (and worse, if the implementation does decide , it does not collect the object until the method is invoked ).  

So - inserting a no-guarantee fallback option seems to doing more harm than good here. 

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657067#action_12657067 ] 

kaykay.unique edited comment on SOLR-914 at 12/16/08 2:01 PM:
--------------------------------------------------------

Separately - it might be worth to wrap around the code with a try .. finally { super.finalize(); } for all the custom finalizers for better code correctness. 

JIRA SOLR-924 logged for the same. Patch submitted for the new jira as well. 

      was (Author: kaykay.unique):
    Separately - it might be worth to wrap around the code with a try .. finally { super.finalize(); } for all the custom finalizers for better code correctness. Let me know what you think about the same.  
  
> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657067#action_12657067 ] 

Kay Kay commented on SOLR-914:
------------------------------

Separately - it might be worth to wrap around the code with a try .. finally { super.finalize(); } for all the custom finalizers for better code correctness. Let me know what you think about the same.  

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kay Kay updated SOLR-914:
-------------------------

    Component/s:     (was: clients - java)
                 Analysis

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: Analysis
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12712576#action_12712576 ] 

Noble Paul commented on SOLR-914:
---------------------------------

what do we plan to do with this? 

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kay Kay updated SOLR-914:
-------------------------

    Priority: Minor  (was: Major)

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: Analysis
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (SOLR-914) Presence of finalize() in the codebase

Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Noble Paul reassigned SOLR-914:
-------------------------------

    Assignee: Noble Paul

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Noble Paul
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Mark Miller (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12711610#action_12711610 ] 

Mark Miller commented on SOLR-914:
----------------------------------

I agree with Kay Kay and Lance here - I don't think we should be doing any closing/shutdown with finalize. Logging a warning seems like the right way to go to me. This is the type of thing that hides problems, and it just doesn't do it cleanly.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12729769#action_12729769 ] 

Kay Kay commented on SOLR-914:
------------------------------

What I meant is (and others who had commented on the jira seem to concur) - logging a warning is ok. 

Code to release resources should be avoided as a finalize is no equivalent to a C++ dtor. 

This patch does not seem to address the issue. 



> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Noble Paul
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666058#action_12666058 ] 

Hoss Man commented on SOLR-914:
-------------------------------

IndexWriter.close() is very cheap *if* the IndexWriter is already closed ... if it's not already closed, then doing so in the finalize() method is our last resort. (but I would think a patch to IndexWriter to make it explicitly implement an "isClosed() method would certainly be nice to help keep the code clean and make it possible to log a warning).

Ditto for ConcurrentLRUCache ... finalize calls destroy which sets the stop flag and notifies the thread ... calling destroy() again shouldn't be that expensive if the client has already called it (which FastLRUCache does) -- but changing the code to record when destroy() has been called, and log a warning in finalize if it hasn't been called yet (then calling it) seems like a good idea.




> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657023#action_12657023 ] 

Kay Kay commented on SOLR-914:
------------------------------

Primarily SolrCore and SolrIndexWriter ( specifically for my use-case ). 

Also - just noticed that - CoreContainer.finalize() ( calls shutdown() ) - has a synchronized block.  While it is not a bottleneck for me , per se, (since I believe all through the life of the web-app , an instance of CoreContainer is alive and reachable , correct me if I am wrong here ). I believe we might need to revisit this if we were to extend this / provide orthogonal integration with other apps. 




> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SOLR-914) Presence of finalize() in the codebase

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hoss Man resolved SOLR-914.
---------------------------

    Resolution: Fixed

Committed revision 807872.


FYI: this patch surfaced some bugs in TestFastLRUCache where the cache wasn't being closed properly.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Hoss Man
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch, SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663540#action_12663540 ] 

Kay Kay commented on SOLR-914:
------------------------------

SolrIndexWriter#finalize() seems to delegate the same to IndexWriter.close() which is quite expensive. 

ConcurrentLRUCache#finalize() seems to close a thread (by means of notify() ) . I am not sure if those methods are good enough candidates to be present in finalize() since they seem to do more than logging at this point. 

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Lance Norskog (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743773#action_12743773 ] 

Lance Norskog commented on SOLR-914:
------------------------------------

Yes, Solr should keep working. But somewhere in the logs (that a few people read) it should note that the resource should have been closed before the finalize. So, yes, the patch is right in that it does the shutdown codes inside try{} blocks that ignore errors.

+1 on on the current patch and the policy it implements.

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Hoss Man
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (SOLR-914) Presence of finalize() in the codebase

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hoss Man reassigned SOLR-914:
-----------------------------

    Assignee: Hoss Man

> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Assignee: Hoss Man
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-914) Presence of finalize() in the codebase

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743562#action_12743562 ] 

Hoss Man commented on SOLR-914:
-------------------------------

I don't understand objections to the idea that finalize should close if (and only it) the resource hasn't already been closed ... people shouldn't relying on it, but having code that aids in the prevention of resource leaks doesn't seem like abad thing to me.

the only things i would change about this patch...
   * make the logging done by the finalizer methods more serious (error or maybe even fatal) and make them convey *why* it's an error ("...was not closed prior to finalizing")
   * SolrIndexWriter.finalize() still calls super.close() (only this.close() should ever call super.close())


> Presence of finalize() in the codebase 
> ---------------------------------------
>
>                 Key: SOLR-914
>                 URL: https://issues.apache.org/jira/browse/SOLR-914
>             Project: Solr
>          Issue Type: Improvement
>          Components: clients - java
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-914.patch
>
>   Original Estimate: 480h
>  Remaining Estimate: 480h
>
> There seems to be a number of classes - that implement finalize() method.  Given that it is perfectly ok for a Java VM to not to call it - may be - there has to some other way  { try .. finally - when they are created to destroy them } to destroy them and the presence of finalize() method , ( depending on implementation ) might not serve what we want and in some cases can end up delaying the gc process, depending on the algorithms. 
> $ find . -name *.java | xargs grep finalize
> ./contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java:  protected void finalize() {
> ./src/java/org/apache/solr/update/SolrIndexWriter.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/CoreContainer.java:  protected void finalize() {
> ./src/java/org/apache/solr/core/SolrCore.java:  protected void finalize() {
> ./src/common/org/apache/solr/common/util/ConcurrentLRUCache.java:  protected void finalize() throws Throwable {
> May be we need to revisit these occurences from a design perspective to see if they are necessary / if there is an alternate way of managing guaranteed destruction of resources. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.