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: dev-unsubscr...@myfaces.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org