You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by GitBox <gi...@apache.org> on 2022/04/27 19:56:37 UTC
[GitHub] [myfaces] volosied opened a new pull request, #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
volosied opened a new pull request, #255:
URL: https://github.com/apache/myfaces/pull/255
https://issues.apache.org/jira/browse/MYFACES-4433
I believe we still want to call ViewScopeContextImpl.destroyAllActive(st); in order to invoke PreDestroy on the ViewScoped beans.
Once that's done, we can remove the contextualStorage from the storageMap
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #255: MYFACES-4433 - 2.2.x - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1144113490
@melloware Sure, I'll do that later today or tomorrow at the latest :)
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
tandraschko commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1111492428
But in general it looks good
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on pull request #255: MYFACES-4433 - 2.2.x - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1144058973
@volosied saw your votes for 2.3, next and 3.0. Any chance you could do a 2.2 release as well? Maybe it will be the last one but it fixes a few significant things.
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #255:
URL: https://github.com/apache/myfaces/pull/255#discussion_r860212512
##########
impl/src/main/java/org/apache/myfaces/cdi/view/ViewScopeBeanHolder.java:
##########
@@ -236,4 +236,8 @@ public String generateUniqueViewScopeId()
return key;
}
+ public void removeStorage(String viewScopeId){
Review Comment:
Maybe rename this to `removeStorageContextFromMap` instead?
Let me know if you have a preference or any other suggestions.
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1112083620
I assume this needs to be forward ported to the higher versions?
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware merged pull request #255: MYFACES-4433 - 2.2.x - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
melloware merged PR #255:
URL: https://github.com/apache/myfaces/pull/255
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
tandraschko commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1112181554
if 2.3 is affected, then also 3.0
and i think if 2.3-next is affected, also 4.0 should be (its a very similar codebase)
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on a diff in pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
melloware commented on code in PR #255:
URL: https://github.com/apache/myfaces/pull/255#discussion_r860215597
##########
impl/src/main/java/org/apache/myfaces/cdi/view/ViewScopeBeanHolder.java:
##########
@@ -236,4 +236,8 @@ public String generateUniqueViewScopeId()
return key;
}
+ public void removeStorage(String viewScopeId){
Review Comment:
I think `removeStorage` is good. Clean and concise
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] melloware commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1111426747
This looks good to me but I prefer @tandraschko give it his blessing :)
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1112179430
It looks like this affects 2.3 and 2.3-next, too. I'll get PRs for them up soon.
4.0 was refactored, so I'm not sure if it is a problem there anymore?
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #255: MYFACES-4433 - 2.2.x - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1112206868
I keep forgetting about 3.0. :(
I got all the PRs up 2.2, 2.3, 2.3-next, and 3.0.
As for 4.0, I found this line:
https://github.com/apache/myfaces/blob/f8a90dd4823b7d2ddb9fbe04eadae59f54cdb330/impl/src/main/java/org/apache/myfaces/cdi/util/AbstractContextualStorageHolder.java#L241
Therefore, I don't think 4.0 is affected by this leak..
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on pull request #255: MYFACES-4433 - Fix ViewScopeContextualStorage memory leak
Posted by GitBox <gi...@apache.org>.
tandraschko commented on PR #255:
URL: https://github.com/apache/myfaces/pull/255#issuecomment-1111491115
Does it only exist in 2.2?
--
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-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org