tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: svn commit: r745898 - in /tomcat/connectors/trunk/jk: native/iis/jk_isapi_plugin.c xdocs/miscellaneous/changelog.xml
Date Wed, 25 Feb 2009 13:26:11 GMT
On 25.02.2009 13:56, Mladen Turk wrote:
> Rainer Jung wrote:
>> On 19.02.2009 16:28, mturk@apache.org wrote:
>>> Author: mturk
>>> Date: Thu Feb 19 15:28:47 2009
>>> New Revision: 745898
>>>
>>> URL: http://svn.apache.org/viewvc?rev=745898&view=rev
>>> Log:
>>> Update uriworkermap on watchog interval
>>>
>>> Modified:
>>> tomcat/connectors/trunk/jk/native/iis/jk_isapi_plugin.c
>>> tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml
>>>
>>> Modified: tomcat/connectors/trunk/jk/native/iis/jk_isapi_plugin.c
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/iis/jk_isapi_plugin.c?rev=745898&r1=745897&r2=745898&view=diff
>>>
>>> ==============================================================================
>>>
>>> --- tomcat/connectors/trunk/jk/native/iis/jk_isapi_plugin.c (original)
>>> +++ tomcat/connectors/trunk/jk/native/iis/jk_isapi_plugin.c Thu Feb
>>> 19 15:28:47 2009
>>> @@ -2377,6 +2377,11 @@
>>> jk_log(logger, JK_LOG_DEBUG,
>>> "Watchdog thread running");
>>> }
>>> + if (worker_mount_file[0]) {
>>> + jk_shm_lock();
>>> + uri_worker_map_update(uw_map, 0, logger);
>>> + jk_shm_unlock();
>>> + }
>>
>> Why the shm lock/unlock? The map itself has a lock that gets used. We
>> don't use the shm lock around map_uri_to_worker_ext(), which also goes
>> down to map_uri_to_worker_ext().
>>
>
> Because IIS6+ can have multiple worker processes.
> map lock is thread lock not shared across childs.

But do those processes share the same map? It is not in shared memory, 
so each process should have it's own copy? Otherwise we would need to 
consider the same for Apache 2.x.

Also as I said, the same problem is already there, when 
map_uri_to_worker_ext() gets called. It can also trigger a map reload 
(as does the status worker).

>> Another thing: do you think it's useful to add this to Apache 2.0 too?
>> What's the reason for it (the map is automatically updated when a
>> request comes in).
>>
>
> Apache has graceful restart, so I doubt this makes sense.

Nevertheless: why is that needed for IIS? Isn't the map automatically 
reloaded if needed?

>>> - uw_map->fname = worker_mount_file;
>>> uw_map->reload = worker_mount_reload;
>>> - if (worker_mount_file[0])
>>> + if (worker_mount_file[0]) {
>>> + uw_map->fname = worker_mount_file;
>>> rc = uri_worker_map_load(uw_map, logger);
>>> + }
>>
>> Moving the uw_map->fname assignment makes it now undefined, in case
>> worker_mount_file is an empty string. Of course that's not a useful
>> worker mount file, but I'm a little worried by letting fname be
>> undefined.
>>
>
> Well, we should check for NULL instead relying the empty ("")
> fname will just fail any ->fname call.

I checked, we always init fname to NULL in uri_worker_map_alloc(), so it 
is well defined. OK.

Regards,

Rainer

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message