[Springframework-rcp-dev] [Spring-JIRA] Commented: (RCP-97) Creating ActionCommands without a CommandFaceDescriptor causes memory leaks

[Jan H. (JIRA) <no...@at...>]

    [ http://opensource.atlassian.com/projects/spring/browse/RCP-97?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_28596 ] 

Jan Hoskens commented on RCP-97:
--------------------------------

After a quick code sweep it seems to me that 2 is the way to go here. 

If at some point you want to change a property of the FaceDescriptor this change should not be reflected by all commands (as is the case now with a shared object). On the other hand not reacting at all (as in 1 and 3)  breaks the purpose of the whole listening aspect. (you might want to change the faceDescriptor at runtime)

> Creating ActionCommands without a CommandFaceDescriptor causes memory leaks
> ---------------------------------------------------------------------------
>
>                 Key: RCP-97
>                 URL: http://opensource.atlassian.com/projects/spring/browse/RCP-97
>             Project: Spring Framework Rich Client Project
>          Issue Type: Improvement
>            Reporter: Calvin Yu
>         Attachments: CommandFaceButtonManager.patch
>
>
> If no CommandFaceDescriptor is specified, then CommandFaceDescriptor.BLANK_FACE_DESCRIPTOR.  The ActionCommand's CommandFaceButtonManager then calls addPropertyChangeListener() on this descriptor, adding itself as the listener.  Unless another CommandFaceDescriptor is set for that ActionCommand, the CommandFaceButtonManager will never be removed from BLANK_FACE_DESCRIPTOR, and therefore the manager and command will never be garbage collected since the descriptor is a static.
> Possible solutions:
> 1) Remove AbstractPropertyChangePublisher's final modifiers on the addPropertyChangeListener() methods, so that they can be overriden by BLANK_FACE_DESCRIPTOR to be a no-op.
> 2) Create a new blank CommandFaceDescriptor instead of using a constant.
> 3) Modify CommandFaceButtonManager so that it isn't added as a PropertyChangeListener if the descriptor is BLANK_FACE_DESCRIPTOR.
> I've attached a patch to solution #3.  While I believe #3 isn't the most optimal solution, it does have less potential side effects to solutions #1 and #2.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/spring/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira