milansie commented on code in PR #705:
URL: https://github.com/apache/myfaces/pull/705#discussion_r1573742064
##########
impl/src/main/java/org/apache/myfaces/push/cdi/WebsocketScopeManager.java:
##########
@@ -161,26 +161,12 @@ public static class ViewScope extends AbstractUserScope
implements Serializable
@Inject private WebsocketScopeManager scopeManager;
@Inject private WebsocketSessionManager sessionManager;
- /*
- * If the view is discarded, destroy the websocket sessions associated
with the view because they are no
- * longer valid
- */
@PreDestroy
public void destroy()
{
- // destroy parent scope ("session")
- SessionScope sessionScope = (SessionScope)
scopeManager.getScope(SCOPE_SESSION, false);
- if (sessionScope != null)
- {
- for (String token : tokens.keySet())
- {
- sessionScope.destroyChannelToken(token);
- }
- }
-
channelTokens.clear();
tokens.clear();
- }
+ }
Review Comment:
As I understand it, channelToken (simple UUID String) is registered (put) in
tokens Map in both, SessionScope and ViewScope bean
(org.apache.myfaces.push.cdi.WebsocketScopeManager.AbstractUserScope#registerToken).
If you create new view (new ViewScope bean), under the same session (same
SessionScope bean), same token is used.
When the ViewScope bean is destroyed, you want to clear tokens for this bean
- that's ok. But you don't want to clear tokens from other View beans, do you?
What happened when you create new view no NUMBER_OF_VIEWS_IN_SESSION+1. You
register channelToken (already existing token) for this ViewScope and
SessionScope (WebsocketComponentRenderer is responsible to that).
Then some mechanism decides, that very first View should be destroyed (View
no1). Method onDestroy is called on ViewScope bean, which clears tokens from
ViewScope (ok), but (in previous implementation) also from SessionScope bean.
And finally, client browser tried to open new WebSocket session (component
is rendered on client side, websockect connection is created from the client
side). But this fails, while it can find sessionScope channelToken, which was
removed in previous step (now I can't remember exactly where if failed, we
faced that issue few weeks ago / but I think it could be easily simulated).
And I think, that our mechanism has nothing to do with real websocket
connection. These are managed by application container (tomcat, ...), and are
creating/destroying automatically (we are just rendering the component, and
reacting to onOpen/onClose events, and of course, we are sending messages).
So, what we do have here is just to handle channel names and channel tokens
linked to websockets connections. When application container is working
correctly, the worst what could happen (IMHO) is having channelTokens reference
to non-existing websocket session until the SessionScope is destroyed.
Otherwise, we have to think about destroying the sessionScope channelTokens
only when we are 100% sure, that no other ViewScope is using that.
I'm ready for discussion :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
