Mladen Turk wrote:
[EMAIL PROTECTED] 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to