Skip to content

Commit 3cccdce

Browse files
hugithordarsonhugithordarson
committed
Release NGComponentRequestHandler's cache entry lock in a finally clause
1 parent 83603ff commit 3cccdce

File tree

1 file changed

+51
-47
lines changed

1 file changed

+51
-47
lines changed

‎ng-appserver/src/main/java/ng/appserver/NGComponentRequestHandler.java‎

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -65,66 +65,70 @@ public NGResponse handleRequest( NGRequest request ) {
6565
throw new IllegalStateException( "The context's session is null. That should never happen" );
6666
}
6767

68-
// Now let's try to restore the page from the cache, using the contextID provided by the URL
69-
// If no page is found (page probably pushed out of the session's page cache), NGPageRestorationException is thrown.
70-
final NGComponent originalPage = session.pageCache().restorePageFromCache( originatingContextID );
68+
// We're executing the following code in a try-block so we can release the lock on the page cache record in the finally clause.
69+
// FIXME: Making the PageCache records use a Lock that implements AutoClosable and obtaining it in a try-with-resources would be really, really nice. We should do that // Hugi 2025-04-06
70+
try {
71+
// Now let's try to restore the page from the cache, using the contextID provided by the URL
72+
// If no page is found (page probably pushed out of the session's page cache), NGPageRestorationException is thrown.
73+
final NGComponent originalPage = session.pageCache().restorePageFromCache( originatingContextID );
7174

72-
// Since we're working with the page we can safely assume it's become relevant again, so we give it another shot at life by moving it to the top of the page cache
73-
session.pageCache().retainPageWithContextIDInCache( originatingContextID );
75+
// Since we're working with the page we can safely assume it's become relevant again, so we give it another shot at life by moving it to the top of the page cache
76+
session.pageCache().retainPageWithContextIDInCache( originatingContextID );
7477

75-
logger.debug( "Page restored from cache is: " + originalPage.getClass() );
78+
logger.debug( "Page restored from cache is: " + originalPage.getClass() );
7679

77-
// Push the page in the context
78-
context.setPage( originalPage );
79-
context.setComponent( originalPage );
80-
context.page().setContextIncludingChildren( request.context() );
80+
// Push the page in the context
81+
context.setPage( originalPage );
82+
context.setComponent( originalPage );
83+
context.page().setContextIncludingChildren( request.context() );
8184

82-
logger.debug( "About to perform takeValuesfromRequest in context {} on page {} ", originatingContextID, originalPage );
85+
logger.debug( "About to perform takeValuesfromRequest in context {} on page {} ", originatingContextID, originalPage );
8386

84-
// We only perform the takeValuesFromRequest phase if there are actual form values to read
85-
if( !request.formValues().isEmpty() ) {
86-
originalPage.takeValuesFromRequest( request, context );
87-
}
87+
// We only perform the takeValuesFromRequest phase if there are actual form values to read
88+
if( !request.formValues().isEmpty() ) {
89+
originalPage.takeValuesFromRequest( request, context );
90+
}
8891

89-
logger.debug( "About to perform invokeAction on element {} in context {} on page {} ", context.senderID(), originatingContextID, originalPage );
92+
logger.debug( "About to perform invokeAction on element {} in context {} on page {} ", context.senderID(), originatingContextID, originalPage );
9093

91-
// We now invoke the action on the original page instance
92-
final NGActionResults actionInvocationResults = originalPage.invokeAction( request, context );
94+
// We now invoke the action on the original page instance
95+
final NGActionResults actionInvocationResults = originalPage.invokeAction( request, context );
9396

94-
logger.debug( "Action invocation returned {}", actionInvocationResults );
97+
logger.debug( "Action invocation returned {}", actionInvocationResults );
9598

96-
// The response we will eventually return
97-
final NGResponse response;
99+
// The response we will eventually return
100+
final NGResponse response;
98101

99-
// The response returned by an action can be
100-
// - null : Meaning we're working within a page/staying on the same page instance
101-
// - An instance of NGComponent : In which case that becomes the new page of this context
102-
// - Everything else that implements NGActionResults : which we just allow to do it's own thing (by invoking generateResponse() on it)
102+
// The response returned by an action can be
103+
// - null : Meaning we're working within a page/staying on the same page instance
104+
// - An instance of NGComponent : In which case that becomes the new page of this context
105+
// - Everything else that implements NGActionResults : which we just allow to do it's own thing (by invoking generateResponse() on it)
103106

104-
if( actionInvocationResults == null ) {
105-
// If an action returns null, we're staying on the same page
106-
logger.debug( "Action method returned null, invoking generateResponse on the original page" );
107-
response = originalPage.generateResponse();
108-
}
109-
else if( actionInvocationResults instanceof NGComponent newPage ) {
110-
// If an action method returns an NGComponent, that's our new page in this context. We set it, and return it
111-
newPage.setContextIncludingChildren( context );
112-
response = newPage.generateResponse();
113-
}
114-
else {
115-
// If this is not an NGComponent, we don't need to take any special action and just invoke generateResponse() on the action's results
116-
response = actionInvocationResults.generateResponse();
117-
}
118-
119-
// Just a little self-documenting sanity checking
120-
if( response == null ) {
121-
throw new IllegalStateException( "Response is null. This should never happen" );
122-
}
107+
if( actionInvocationResults == null ) {
108+
// If an action returns null, we're staying on the same page
109+
logger.debug( "Action method returned null, invoking generateResponse on the original page" );
110+
response = originalPage.generateResponse();
111+
}
112+
else if( actionInvocationResults instanceof NGComponent newPage ) {
113+
// If an action method returns an NGComponent, that's our new page in this context. We set it, and return it
114+
newPage.setContextIncludingChildren( context );
115+
response = newPage.generateResponse();
116+
}
117+
else {
118+
// If this is not an NGComponent, we don't need to take any special action and just invoke generateResponse() on the action's results
119+
response = actionInvocationResults.generateResponse();
120+
}
123121

124-
// FIXME: If we have a checked out page instance, we probably need to release the lock in a finally clause // Hugi 2025-04-06
125-
session.pageCache().releaseLock( originatingContextID );
122+
// Just a little self-documenting sanity checking
123+
if( response == null ) {
124+
throw new IllegalStateException( "Response is null. This should never happen" );
125+
}
126+
return response;
126127

127-
return response;
128+
}
129+
finally {
130+
session.pageCache().releaseLock( originatingContextID );
131+
}
128132
}
129133

130134
/**

0 commit comments

Comments
 (0)