[jira] Created: (JCR-2378) Avoid exceptions thrown in finalize handler of RepositoryImpl constructor
|
|
JIRA jira@apache.org
|
Avoid exceptions thrown in finalize handler of RepositoryImpl constructor
------------------------------------------------------------------------- Key: JCR-2378 URL: https://issues.apache.org/jira/browse/JCR-2378 Project: Jackrabbit Content Repository Issue Type: Improvement Components: jackrabbit-core Affects Versions: 2.0-alpha11 Reporter: Alexander Klimetschek Priority: Minor If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: } finally { if (!succeeded) { try { // repository startup failed, clean up... shutdown(); } catch (Throwable t) { // shutdown() likely to fail now, as startup was broken... log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); } } } [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup [2] Overlaying exception's stacktrace: Caused by: java.lang.NullPointerException at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
|
JIRA jira@apache.org
|
[ https://issues.apache.org/jira/browse/JCR-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alexander Klimetschek updated JCR-2378: --------------------------------------- Attachment: JCR-2378.patch Patch that both fixes the NPE in ObservationManagerImpl (throws early now) and adds the try/catch to the finalizer. > Avoid exceptions thrown in finalize handler of RepositoryImpl constructor > ------------------------------------------------------------------------- > > Key: JCR-2378 > URL: https://issues.apache.org/jira/browse/JCR-2378 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core > Affects Versions: 2.0-alpha11 > Reporter: Alexander Klimetschek > Priority: Minor > Attachments: JCR-2378.patch > > > If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. > This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. > I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: > } finally { > if (!succeeded) { > try { > // repository startup failed, clean up... > shutdown(); > } catch (Throwable t) { > // shutdown() likely to fail now, as startup was broken... > log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); > } > } > } > [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup > [2] Overlaying exception's stacktrace: > Caused by: java.lang.NullPointerException > at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) > at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) > at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) > at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) > at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) > at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) > at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) > at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) > at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) > at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) > ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
|
JIRA jira@apache.org
|
In reply to this post
by JIRA jira@apache.org
[ https://issues.apache.org/jira/browse/JCR-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alexander Klimetschek updated JCR-2378: --------------------------------------- Assignee: Alexander Klimetschek Status: Patch Available (was: Open) Patch available. > Avoid exceptions thrown in finalize handler of RepositoryImpl constructor > ------------------------------------------------------------------------- > > Key: JCR-2378 > URL: https://issues.apache.org/jira/browse/JCR-2378 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core > Affects Versions: 2.0-alpha11 > Reporter: Alexander Klimetschek > Assignee: Alexander Klimetschek > Priority: Minor > Attachments: JCR-2378.patch > > > If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. > This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. > I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: > } finally { > if (!succeeded) { > try { > // repository startup failed, clean up... > shutdown(); > } catch (Throwable t) { > // shutdown() likely to fail now, as startup was broken... > log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); > } > } > } > [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup > [2] Overlaying exception's stacktrace: > Caused by: java.lang.NullPointerException > at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) > at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) > at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) > at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) > at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) > at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) > at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) > at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) > at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) > at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) > ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
|
JIRA jira@apache.org
|
In reply to this post
by JIRA jira@apache.org
[ https://issues.apache.org/jira/browse/JCR-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772539#action_12772539 ] Jukka Zitting commented on JCR-2378: ------------------------------------ +0 I would prefer to see a solution to the root cause (shutdown not working properly when repository startup fails) instead of a workaround, but until such a solution is available I think doing something like this is a good alternative. > Avoid exceptions thrown in finalize handler of RepositoryImpl constructor > ------------------------------------------------------------------------- > > Key: JCR-2378 > URL: https://issues.apache.org/jira/browse/JCR-2378 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core > Affects Versions: 2.0-alpha11 > Reporter: Alexander Klimetschek > Assignee: Alexander Klimetschek > Priority: Minor > Attachments: JCR-2378.patch > > > If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. > This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. > I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: > } finally { > if (!succeeded) { > try { > // repository startup failed, clean up... > shutdown(); > } catch (Throwable t) { > // shutdown() likely to fail now, as startup was broken... > log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); > } > } > } > [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup > [2] Overlaying exception's stacktrace: > Caused by: java.lang.NullPointerException > at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) > at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) > at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) > at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) > at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) > at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) > at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) > at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) > at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) > at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) > ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
|
JIRA jira@apache.org
|
In reply to this post
by JIRA jira@apache.org
[ https://issues.apache.org/jira/browse/JCR-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772546#action_12772546 ] Alexander Klimetschek commented on JCR-2378: -------------------------------------------- > I would prefer to see a solution to the root cause Yes, but I think there is too much involved to capture all possible problems (easily). Therefore I think this is just good defensive programming. I'll change the wording and add "unexpected" rather than "likely to fail". Also created a separate issue for the NPE: JCR-2380 > Avoid exceptions thrown in finalize handler of RepositoryImpl constructor > ------------------------------------------------------------------------- > > Key: JCR-2378 > URL: https://issues.apache.org/jira/browse/JCR-2378 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core > Affects Versions: 2.0-alpha11 > Reporter: Alexander Klimetschek > Assignee: Alexander Klimetschek > Priority: Minor > Attachments: JCR-2378.patch > > > If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. > This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. > I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: > } finally { > if (!succeeded) { > try { > // repository startup failed, clean up... > shutdown(); > } catch (Throwable t) { > // shutdown() likely to fail now, as startup was broken... > log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); > } > } > } > [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup > [2] Overlaying exception's stacktrace: > Caused by: java.lang.NullPointerException > at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) > at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) > at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) > at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) > at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) > at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) > at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) > at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) > at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) > at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) > ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
|
JIRA jira@apache.org
|
In reply to this post
by JIRA jira@apache.org
[ https://issues.apache.org/jira/browse/JCR-2378?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alexander Klimetschek updated JCR-2378: --------------------------------------- Resolution: Fixed Fix Version/s: 2.0.0 Status: Resolved (was: Patch Available) Added the finalizer change in revision 831932. > Avoid exceptions thrown in finalize handler of RepositoryImpl constructor > ------------------------------------------------------------------------- > > Key: JCR-2378 > URL: https://issues.apache.org/jira/browse/JCR-2378 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core > Affects Versions: 2.0-alpha11 > Reporter: Alexander Klimetschek > Assignee: Alexander Klimetschek > Priority: Minor > Fix For: 2.0.0 > > Attachments: JCR-2378.patch > > > If an exception happens during initialization of the repository, it might be overlayed by an exception thrown in the finalize handler of the RepositoryImpl constructor (see line 382 ff in [1]). The latter exception wins and the original exception is lost (if you don't have a log). This makes it hard to figure out the real problem. > This problem is actually a bit self-enforcing: if something goes wrong during startup, the code in the shutdown() method that is called is actually very prone to fail as it might not expect such a broken-startup state. In my case the overlaying NPE happened in ObservationManagerImpl.getRegisteredEventListeners, where this.dispatcher was unexpectedly null [2]. > I think both places should be fixed (NPE guard in ObservationManagerImpl constructor for "dispatcher") and a try/catch block in the finalizer, just logging the exception: > } finally { > if (!succeeded) { > try { > // repository startup failed, clean up... > shutdown(); > } catch (Throwable t) { > // shutdown() likely to fail now, as startup was broken... > log.error("In addition to startup fail, another problem occurred while shutting down the repository again.", e); > } > } > } > [1] http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/RepositoryImpl.java?view=markup > [2] Overlaying exception's stacktrace: > Caused by: java.lang.NullPointerException > at org.apache.jackrabbit.core.observation.ObservationManagerImpl.getRegisteredEventListeners(ObservationManagerImpl.java:143) > at org.apache.jackrabbit.core.SessionImpl.removeRegisteredEventListeners(SessionImpl.java:1190) > at org.apache.jackrabbit.core.SessionImpl.logout(SessionImpl.java:1215) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.doDispose(RepositoryImpl.java:2153) > at com.day.crx.core.CRXRepositoryImpl$CRXWorkspaceInfo.doDispose(CRXRepositoryImpl.java:1095) > at org.apache.jackrabbit.core.RepositoryImpl$WorkspaceInfo.dispose(RepositoryImpl.java:2108) > at org.apache.jackrabbit.core.RepositoryImpl.doShutdown(RepositoryImpl.java:1146) > at com.day.crx.core.CRXRepositoryImpl.doShutdown(CRXRepositoryImpl.java:845) > at org.apache.jackrabbit.core.RepositoryImpl.shutdown(RepositoryImpl.java:1098) > at org.apache.jackrabbit.core.RepositoryImpl.<init>(RepositoryImpl.java:387) > at com.day.crx.core.CRXRepositoryImpl.<init>(CRXRepositoryImpl.java:201) > at com.day.crx.core.CRXRepositoryImpl.create(CRXRepositoryImpl.java:190) > ... 28 more -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |
