@PreDestroy not called all the times on view scoped using CDI

[rmartinc]

Hi!

In the last version 2.3.14 a CDI bean annotated with @PreDestroy

is not called all the times that are needed if there are more views in the active_views (refresh).

To reproduce the issue:

1. Deploy the bean.war resulting of a mvn package of the attached maven project in a server with the last version of mojarra 2.3.14.
2. Go to the index page: http://localhost:8080/bean/
3. A first count bean is created.
4. Refresh the page (F5). A new bean is created.
5. Invalidate the session. Only one bean is called for destroy (the last one). If you refresh more times it's the same. Only the last one is destroyed.

Best regards!

bean.zip

[rmartinc]

Looking the issue it seems that it is related to this commit:
javaee/mojarra@eb570d7

The reason is that now the context views can be joined (method copyViewScopeContextsFromSession in ViewScopeContextManager.java) if there is a bean with the same name in the view (like in our case). For what I understand this was done to fix replication issues, but maybe the root problem is using a hashCode of the view map to index the ACTIVE_VIEW_CONTEXTS map. If instead of a hashCode a UUID is used (like the ACTIVE_VIEW_MAPS is doing), the session replication will maintain the indexes OK.

I have done a tentative fix in my branch against 2.3. Check:
2.3...rmartinc:issue4646

But the problem is that a public method signature should be changed (to pass the viewMapId to the managers). Both classes are from the implementation, but I'm not sure if you want something like this. But I really don't see an easy solution to pass the fixed index that maps the view map with the context map.

Please check and comment if you like the solution or if you see something better for this.

Thanks a lot!

[rmartinc]

@arjantijms Please, can you take a look?

[rmartinc]

Hi, did you have the chance of checking the branch? Good or bad idea?

Thanks a lot!

[rmartinc]

Hi,

Sorry for bothering you again but we need to know how to continue with this issue. Thinking about the problem I see only two solutions to get a fixed key for the ACTIVE_VIEW_CONTEXTS map (one key that is replicated OK in a cluster env):

1. Passing the ID as I'm doing now in my branch and using the same UUID the ACTIVE_VIEW_MAPS is using. PROs: it's easy and use the same idea for both maps. CONs: Needs a modification in the public method clear in ViewScopeManager and ViewScopeContextManager.
2. Modifying the ViewMap that is returned to have something that can be used as an ID. Maybe we can make the class public (we can cast to use a new getId method) or implement a special get and put method to retrieve and set the ID (this way the ID can be obtained by the managers from the ViewMap). PROs: we don't need to modify the public methods. CONs: We need to perform some changes in the UIViewRoot which is part of the specification (although those changes can avoid the API breakage).

Don't know, I don't see any other possibility. Can you please check this? Do you see any other solution? Which one of the previous two do you prefer?

I will work in the solution you prefer or a new one if you see something better.

[rmartinc]

Hi,

We have sent a PR: #4655

Finally we decided to be cautious and maintain the API compatibility. Please, as I commented in the PR, take a look when you have time because we have a customer pushing for a solution in our side.

If you find any problem or improvement just request changes in the PR.

Thanks in advance!

[rmartinc]

@arjantijms I have back-ported the fix for 2.3 and 3.0 branch. At least we are interested in including this in the current 2.3 branch (plain cherry-pick was done and test working). Nevertheless I have also sent the same for 3.0 just in case, although I see that tests are not there for the moment (here minor modifications were needed).

• PR for 2.3: [Issue 4646] [branch 2.3] @PreDestroy not called all the times on view scoped using CDI #4909
• PR for 3.0: [Issue 4646] [branch 3.0] @PreDestroy not called all the times on view scoped using CDI #4910

Thanks a lot for working on this!

[BalusC]

Excellent.

But the problem is that a public method signature should be changed (to pass the viewMapId to the managers). Both classes are from the implementation, but I'm not sure if you want something like this.

Not a problem at all as long done on impl side!