[MAGNOLIA-6363] Unclosed sessions on doInSystemContext op.exec() failures Created: 25/Aug/15  Updated: 15/Apr/16  Resolved: 18/Nov/15

Status: Closed
Project: Magnolia
Component/s: None
Affects Version/s: None
Fix Version/s: 5.3.11, 5.4.2

Type: Bug Priority: Critical
Reporter: Milan Divilek Assignee: Evzen Fochr
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 7h
Original Estimate: Not Specified

Attachments: Text File Prevent_NPEs_on_GuiceUtil.patch    
Issue Links:
Relates
relates to MAGNOLIA-6445 Don't swallow exceptions in Op.exec()... Open
relation
is related to MGNLSCH-60 Unclosed sessions on job failures Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Sprint (Kromeriz) 8
Story Points: 2

 Description   

When op fails during the execution, sessions are not closed properly resulting in errors in log files like:

2015-08-28 14:34:36,421 WARN  org.apache.jackrabbit.core.SessionImpl            : Unclosed session detected. The session was opened here: 
java.lang.Exception: Stack Trace
	at org.apache.jackrabbit.core.SessionImpl.<init>(SessionImpl.java:222)
	at org.apache.jackrabbit.core.SessionImpl.<init>(SessionImpl.java:239)
	at org.apache.jackrabbit.core.XASessionImpl.<init>(XASessionImpl.java:101)
	at org.apache.jackrabbit.core.RepositoryImpl.createSessionInstance(RepositoryImpl.java:1613)
	at org.apache.jackrabbit.core.RepositoryImpl.createSession(RepositoryImpl.java:956)
	at org.apache.jackrabbit.core.RepositoryImpl.login(RepositoryImpl.java:1501)
	at org.apache.jackrabbit.core.jndi.BindableRepository.login(BindableRepository.java:162)
	at info.magnolia.jackrabbit.ProviderImpl.getSystemSession(ProviderImpl.java:501)
	at info.magnolia.repository.DefaultRepositoryManager.getSystemSession(DefaultRepositoryManager.java:298)
	at info.magnolia.context.SystemRepositoryStrategy.internalGetSession(SystemRepositoryStrategy.java:54)
	at info.magnolia.context.AbstractRepositoryStrategy.getSession(AbstractRepositoryStrategy.java:75)
	at info.magnolia.context.AbstractContext.getJCRSession(AbstractContext.java:132)
	at info.magnolia.context.AbstractContext.getHierarchyManager(AbstractContext.java:207)
	at info.magnolia.module.ModuleManagerImpl.getModuleInstanceProperties(ModuleManagerImpl.java:422)
	at info.magnolia.module.ModuleManagerImpl$2$1.doExec(ModuleManagerImpl.java:360)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:421)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:418)
	at info.magnolia.context.MgnlContext.doInSystemContext(MgnlContext.java:392)
	at info.magnolia.module.ModuleManagerImpl$2.onEvent(ModuleManagerImpl.java:356)
	at info.magnolia.cms.util.ObservationUtil$ObservationBasedDelayedExecutor$1.run(ObservationUtil.java:253)
	at info.magnolia.cms.util.DelayedExecutor$RunnableWrapper.run(DelayedExecutor.java:103)
	at EDU.oswego.cs.dl.util.concurrent.ClockDaemon$RunLoop.run(Unknown Source)
	at java.lang.Thread.run(Thread.java:745)


 Comments   
Comment by Evzen Fochr [ 01/Sep/15 ]

It happened in MgnlContext#doInSystemContext, releaseAfterExecution part wasn't in finally block so if Op failed with exception it only set MgnlContext to original context.

Comment by Magnolia International [ 15/Oct/15 ]

When an exception occurs in Op.exec() or even potentially in MgnlContext.getSystemContext(), AND MgnlContext.release() ALSO fails, the former exception is lost; neither logged nor thrown.

IMO, the former exception is the one we want to be throwing, and a further exception in release should be ignored (but logged anyway)

This happens a lot, btw, especially during development, since we start components/guice through a doInSystemContext call from MagnoliaServletContextListener; any misconfigured or missing component, wrong constructor, etc, just results in NPE in info.magnolia.objectfactory.guice.GuiceUtils#hasExplicitBindingFor because of this.

cmeier could you please attach the patch we did to GuiceUtil on your machine ? I believe if this error here gets fixed, the NPE won't happen again, however, having an explicit exception rather than an NPE would have helped as well.

Comment by Magnolia International [ 15/Oct/15 ]

Also, I would question the test you added here. It seems overly complicated and undocumented. At the very least, it should say what it does/why (i understand there's probably no easy to test those pesky static methods)

Comment by Evzen Fochr [ 15/Oct/15 ]

Hi, fix Version/s: 5.3.11, 5.4.2 for this ticket was already released. If you want some changes, can/should we do it in new ticket?

Comment by Evzen Fochr [ 18/Nov/15 ]

Created followup ticket MAGNOLIA-6445

Generated at Mon Feb 12 04:13:49 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.