[#UP-745] ChannelManager has a memory leak, when it swaps out a channel for the CError channel, the end session events never progagate to the original channel

uPortal

ChannelManager has a memory leak, when it swaps out a channel for the CError channel, the end session events never progagate to the original channel

Created: 13/Nov/04 07:35 AM Updated: 03/Jul/07 04:10 PM

Component/s:

Framework

Affects Version/s:

2.4.1

Fix Version/s:

2.4.2

Original Estimate:	Unknown	Remaining Estimate:	Unknown	Time Spent:	Unknown
Environment:	« Hide Operating System: All Platform: All Show » Operating System: All Platform: All

Bugzilla Id:

1789

Description

« Hide

> 2) When the ChannelManager swaps out a channel for the CError channel, the
> end session events never propagate to the original channel. This was
> particularly bad with the CPortletChannel channel which keeps a static Map
> for ChannelState objects. Luckily all my portlets on the "Portlet Examples"
> tab weren't configurated and all failed .. which uncovered it.
>
> We could add a method to CError to retrieve the "the_channel" attribute,
> then have ChannelManager propagate the SESSION_DONE event (or others??).

All

Comments

Work Log

Change History

Sort Order:

[ Permlink | « Hide ]

Nick Bolton [13/Nov/04 09:30 AM]

There is similar behavor with CSecureInfo. Both CSecureInfo and CError should
implement receiveEvent and pass events they receive along to the channels they
hold.

[ Permlink | « Hide ]

Andrew Petro [14/Nov/04 05:05 PM]

I saw a commit on this in uP2HEAD to enhance
org.jasig.portal.channels.error.CError to implement receiveEvent() and pass
received events to underlying channels when they are present.

I think org.jasig.portal.channels.CError, the legacy delegating stub in the
HEAD, also needs to implement receiveEvent() and pass the event to its
org.jasig.portal.channels.error.CError delegate.

I can make this change to org.jasig.portal.channels.CError in the HEAD if desired.

[ Permlink | « Hide ]

William G. Thompson, Jr. [14/Nov/04 07:52 PM]

This is how I was able to reproduce the CError bug using uP 2.4.1
1) deploy uP
2) remove all the portlet webapps (this will force all the portlets to instead
render the CError channel)
3) login as admin (they had the example portlet tab as their default)
4) logout, force GC, take a memory snapshot using yourkit profiler

On each logout, you should see +1 ChannelManager objects.

[ Permlink | « Hide ]

William G. Thompson, Jr. [14/Nov/04 07:53 PM]

Bill,

I've verified that adding the receiveEvent method to CError channel fixes
the CM leak.

public void receiveEvent(PortalEvent ev) {
if (the_channel != null) { the_channel.receiveEvent(ev); }
super.receiveEvent(ev);
}

Let me know if that works for you too.

-nick

[ Permlink | « Hide ]

William G. Thompson, Jr. [14/Nov/04 07:54 PM]

Ok. I was able verified both the bug and your fix. Nice work!

later.
Bill

[ Permlink | « Hide ]

Andrew Petro [14/Nov/04 08:13 PM]

My bad, my previous post was wrong. The legacy CError stub already passes
events along to its delegate. Nick's commit was exactly correct.
Congratulations on plugging this leak.

[I saw a commit on this in uP2HEAD to enhance
org.jasig.portal.channels.error.CError to implement receiveEvent() and pass
received events to underlying channels when they are present.

I think org.jasig.portal.channels.CError, the legacy delegating stub in the
HEAD, also needs to implement receiveEvent() and pass the event to its
org.jasig.portal.channels.error.CError delegate.

I can make this change to org.jasig.portal.channels.CError in the HEAD if desired.]