tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <>
Subject Re: [PROPOSAL] Manager behaviour clarification for 9.0.x
Date Fri, 08 Jan 2016 18:08:57 GMT
2016-01-08 20:26 GMT+03:00 Mark Thomas <>:
> On 08/01/2016 16:56, Konstantin Kolinko wrote:
>> 2016-01-08 15:33 GMT+03:00 Mark Thomas <>:
>>> I've been looking at the relationship between a Context and its Manager
>>> and I have identified some inconsistencies in the underlying assumptions
>>> on which the code is based.
>>> A. Context is always set
>>> In some places the code carefully checks to see if the Context is null.
>>> In other places (often in the same method and before the null check) the
>>> code assumes that the Context is non-null.
>> Specific examples = ?   I see only a few checks
> null check vs no null check:
> [File|JDBC]Store before r1723736

I guess I was looking at the current implementation after your fix.

OK, those are unusable outside of a web application.

Or it makes sense to use them offline?
- to read session data from a standalone application,
- in a test

> Assumes non-null:
> ManagerBase
>   getObjectNameKeyProperties()
>   getDomainInternal()
>   SingleSignOn
>   SingleSignOnSessionKey


> Checks for null
>   setContext()
>   toString()

The null checks are needed in both these places. The toString() can be
called at any time.

>   SingleSingOnEvent

I think you mean SingleSignOnListener.sessionEvent().

A sanity check. Not really needed. But that method is full of sanity
checks, so one more does not hurt.

>> [...]

>> The default implementation can be wrong and one should be able to
>> replace it. That is usually performed by a LifecycleListener.
>> If so, Context shall call setContext(null) on the old manager instance
>> so that the old Manager unregisters itself as PropertyChangeListener
>> on the context.
>> Actually currently there is no such setContext(null) call, which is a bug.
> We can fix that.

> In summary, what I am intending is:
> Context.manager -> change whenever you like although change while
> running at your own risk
> Manager.context -> no changes once the Manager is not in the NEW state.
> I hope this clarifies what I was proposing.


A call to context.removePropertyChangeListener() is needed .

Instead of setContext(null) call  it will be more clear to add the
listener in initInternal() and to remove it in destroyInternal().

Best regards,
Konstantin Kolinko

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message