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: r603637 - /tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c
Date Wed, 12 Dec 2007 16:02:39 GMT
Mladen Turk wrote:
> rjung@apache.org wrote:
>> Author: rjung
>> Date: Wed Dec 12 07:10:32 2007
>> New Revision: 603637
>>
>> URL: http://svn.apache.org/viewvc?rev=603637&view=rev
>> Log:
>> Slightly rearange ajp_next_connection().
>>
> 
>>      int rc;
>>      ajp_worker_t *aw = ae->worker;
>> -    jk_sock_t sock;
>>  
>> +    /* Close previous socket */
>> +    if (IS_VALID_SOCKET(ae->sd))
>> +        jk_shutdown_socket(ae->sd, l);
>> +    /* Mark existing endpoint socket as closed */
>> +    ae->sd = JK_INVALID_SOCKET;
>>      JK_ENTER_CS(&aw->cs, rc);
>>      if (rc) {
>>          unsigned int i;
>> -        sock = ae->sd;
>> -        /* Mark existing endpoint socket as closed */
>> -        ae->sd = JK_INVALID_SOCKET;
> 
> Hmm, I'm almost sure this is wrong.
> It makes race condition in threaded servers.
> shutdown is blocking call.
> Any particular reason why you changed that?

The shutdown is done on the socket belonging to the endpoint, so it is 
not in concurrent use (the endpoint got it's socket for private usage 
before we called this function).

So synchronization should not be necessary for the shutdown and thus the 
blocking nature of the shutdown is not evil.

In fact the only reason I moved it to the front was:

- I think it is allowed to move it outside of the lock
- If we then move it in front, we don't need to first save the socket to 
a temporary variable only to destroy it at the end. We can destroy directly.

Anything wrong with this reasoning?

> Regards,
> Mladen

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