Re: [uportal-dev] SmartLdap thread leak

[Eric Dalquist]

Sorry, I miss-read the code and thought there was essentially a while(true) loop in the refresh code that stayed resident as a background thread.

-Eric

On 8/6/10 11:34 AM, Drew Wills wrote:

On 8/6/2010 11:10 AM, Cris J Holdorph wrote:

What do you mean it will leak the refresh thread?  Won't the thread
complete and then die? I didn't think the thread lived on forever, but
only for as long as it took to refresh the group tree one time.

This is my understanding as well -- the thread completes & dies. It's a daemon thread, moreover, so it won't interfere with shutting down the JVM.

But I agree, if quartz is available, that's what quartz is good at and
I'd strongly advocate for using it instead. I'm not sure how I missed
knowing that got added to uPortal in the past. *shrug* Now I do.

So... I like Quartz too. I understand these points, and I considered setting it up with the scheduler right off the bat. But I had some counter-points in mind too, which led me to think we might be better off with the alternative. SmartLdap is an optional component of uPortal. It's used by a minority of deployments. I dislike it when (and I think new adopters find it intimidating when) it takes changes in 2-3 separate config files to turn on 1 logical option. We actually have several components where this is the case. Consider the CacheSecurityContext/password replay tech: https://wiki.jasig.org/x/EIq8AQ

Enabling this (one) option requires changes to 3 files:
  - security.properties
  - portletContainerContext.xml
  - securityContext.xml







But I wanted to make it clear:
  - (1) I don't believe it leaks threads;  and


drew wills

Actually the last of these wasn't even documented until the other day, when a new colleague of mine discovered that the documented steps for enabling this feature weren't working. To me, the tree-refresh feature of SmartLdap isn't something that needs to be complicated. You can configure how frequently it happens or shut it off entirely... all in the same file in which you configure the rest of it. There's even a sensible default behavior, so if you just drop your config from an earlier version into the new version, SmartLdap doesn't care and will do something reasonable. So then... I'm ok with changing it per Eric's suggestions. I'm happy for the uP devs to consider the question, form a consensus, and take action. I'd actually love to see more contributors in the code. - (2) The current setup was a conscious, reasoned choice, not an oversight or omission

---- Cris J H

On 08/06/2010 10:59 AM, Eric Dalquist wrote:

The change set for SmartLdap will leak the refresh thread when the
portal webapp is reloaded.


the quartz scheduler that already exists in 3.x. Using the schedule

of the refresh rate.

https://developer.jasig.org/source/changelog/jasigsvn/uPortal?cs=21422

-Eric

If possible this refresh process should be a periodic trigger managed by guarantees correct thread handling and configuration level manageability

smime.p7s
Description: S/MIME Cryptographic Signature