You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by rj...@apache.org on 2007/12/14 20:19:18 UTC

svn commit: r604267 - /tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c

Author: rjung
Date: Fri Dec 14 11:19:17 2007
New Revision: 604267

URL: http://svn.apache.org/viewvc?rev=604267&view=rev
Log:
Various minor cleanups + function comments.

Modified:
    tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c

Modified: tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c?rev=604267&r1=604266&r2=604267&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c Fri Dec 14 11:19:17 2007
@@ -717,13 +717,17 @@
 }
 
 
-/*
- * Try another connection from cache
+/** Steal a connection from an idle cache endpoint
+ * @param ae   endpoint that needs a new connection
+ * @param l    logger
+ * @return     JK_FALSE: failure
+ *             JK_TRUE: success
+ * @remark     Always closes old socket endpoint
  */
-
-static void ajp_next_connection(ajp_endpoint_t *ae, jk_logger_t *l)
+static int ajp_next_connection(ajp_endpoint_t *ae, jk_logger_t *l)
 {
     int rc;
+    int ret = JK_FALSE;
     ajp_worker_t *aw = ae->worker;
 
     JK_TRACE_ENTER(l);
@@ -745,17 +749,25 @@
             }
         }
         JK_LEAVE_CS(&aw->cs, rc);
-        if (IS_VALID_SOCKET(ae->sd))
+        if (IS_VALID_SOCKET(ae->sd)) {
+            ret = JK_TRUE;
             jk_log(l, JK_LOG_INFO,
                    "(%s) Will try pooled connection sd = %d from slot %d",
                     ae->worker->name, ae->sd, i);
+        }
     }
     JK_TRACE_EXIT(l);
+    return ret;
 }
 
-/*
- * Handle the CPING/CPONG initial query
- * Cares about ae->errno.
+/** Handle the cping/cpong query
+ * @param ae       endpoint
+ * @param timeout  wait timeout in milliseconds
+ * @param l        logger
+ * @return         JK_FALSE: failure
+ *                 JK_TRUE: success
+ * @remark         Always closes socket in case of
+ *                 a socket error
  */
 static int ajp_handle_cping_cpong(ajp_endpoint_t * ae, int timeout, jk_logger_t *l)
 {
@@ -793,7 +805,7 @@
      */
     if (jk_is_input_event(ae->sd, timeout, l) == JK_FALSE) {
         ae->last_errno = errno;
-        jk_log(l, JK_LOG_INFO, "timeout in reply pong");
+        jk_log(l, JK_LOG_INFO, "timeout in reply cpong");
         /* We can't trust this connection any more. */
         jk_shutdown_socket(ae->sd, l);
         ae->sd = JK_INVALID_SOCKET;
@@ -811,9 +823,12 @@
     }
 
     if ((cmd = jk_b_get_byte(msg)) != AJP13_CPONG_REPLY) {
-        jk_log(l, JK_LOG_INFO,
+        jk_log(l, JK_LOG_ERROR,
                "awaited reply cpong, received %d instead",
                cmd);
+        /* We can't trust this connection any more. */
+        jk_shutdown_socket(ae->sd, l);
+        ae->sd = JK_INVALID_SOCKET;
         JK_TRACE_EXIT(l);
         return JK_FALSE;
     }
@@ -822,9 +837,14 @@
     return JK_TRUE;
 }
 
-/*
- * Connect an endpoint to a backend.
- * Cares about ae->errno.
+/** Connect an endpoint to a backend
+ * @param ae       endpoint
+ * @param l        logger
+ * @return         JK_FALSE: failure
+ *                 JK_TRUE: success
+ * @remark         Always closes socket in case of
+ *                 a socket error
+ * @remark         Cares about ae->last_errno
  */
 int ajp_connect_to_endpoint(ajp_endpoint_t * ae, jk_logger_t *l)
 {
@@ -841,7 +861,7 @@
 
     if (!IS_VALID_SOCKET(ae->sd)) {
         ae->last_errno = errno;
-        jk_log(l, JK_LOG_INFO,
+        jk_log(l, JK_LOG_ERROR,
                "Failed opening socket to (%s) (errno=%d)",
                jk_dump_hinfo(&ae->worker->worker_inet_addr, buf), ae->last_errno);
         JK_TRACE_EXIT(l);
@@ -863,36 +883,38 @@
     /* XXX: and if no cping/cpong is allowed before or after logon. */
     if (ae->worker->logon != NULL) {
         rc = ae->worker->logon(ae, l);
-        if (rc == JK_FALSE)
-            jk_log(l, JK_LOG_INFO,
+        if (rc == JK_FALSE) {
+            jk_log(l, JK_LOG_ERROR,
                    "(%s) ajp14 worker logon to the backend server failed",
                    ae->worker->name);
+            /* Close the socket if unable to logon */
+            jk_shutdown_socket(ae->sd, l);
+            ae->sd = JK_INVALID_SOCKET;
+        }
     }
-    /* should we send a CPING to validate connection ? */
+    /* XXX: Should we send a cping also after logon to validate the connection? */
     else if (ae->worker->connect_timeout > 0) {
         rc = ajp_handle_cping_cpong(ae, ae->worker->connect_timeout, l);
         if (rc == JK_FALSE)
-            jk_log(l, JK_LOG_INFO,
+            jk_log(l, JK_LOG_ERROR,
                    "(%s) cping/cpong after connecting to the backend server failed (errno=%d)",
                    ae->worker->name, ae->last_errno);
     }
-    if (rc != JK_TRUE) {
-        /* Close the socket if unable to connect */
-        jk_log(l, JK_LOG_INFO,
-               "(%s) error connecting to the backend server",
-               ae->worker->name, ae->last_errno);
-        jk_shutdown_socket(ae->sd, l);
-        ae->sd = JK_INVALID_SOCKET;
-    }
     JK_TRACE_EXIT(l);
     return rc;
 }
 
-/*
- * Send a message to endpoint, using corresponding PROTO HEADER
- * Cares about ae->errno.
+/** Send a message to an endpoint, using corresponding PROTO HEADER
+ * @param ae       endpoint
+ * @param msg      message to send
+ * @param l        logger
+ * @return         JK_FATAL_ERROR: message contains unknown protocol
+ *                 JK_FALSE: other failure 
+ *                 JK_TRUE: success
+ * @remark         Always closes socket in case of
+ *                 a socket error, or JK_FATAL_ERROR
+ * @remark         Cares about ae->last_errno
  */
-
 int ajp_connection_tcp_send_message(ajp_endpoint_t * ae,
                                     jk_msg_buf_t *msg, jk_logger_t *l)
 {
@@ -915,6 +937,11 @@
         jk_log(l, JK_LOG_ERROR,
                "(%s) unknown protocol %d, supported are AJP13/AJP14",
                 ae->worker->name, ae->proto);
+        /* We've got a protocol error. */
+        /* We can't trust this connection any more, */
+        /* because we might have send already parts of the request. */
+        jk_shutdown_socket(ae->sd, l);
+        ae->sd = JK_INVALID_SOCKET;
         JK_TRACE_EXIT(l);
         return JK_FATAL_ERROR;
     }
@@ -938,11 +965,16 @@
     return JK_FALSE;
 }
 
-/*
- * Receive a message from endpoint, checking PROTO HEADER
- * Cares about ae->errno.
+/** Receive a message from an endpoint, checking PROTO HEADER
+ * @param ae       endpoint
+ * @param msg      message to send
+ * @param l        logger
+ * @return         JK_FALSE: failure 
+ *                 JK_TRUE: success
+ * @remark         Always closes socket in case of
+ *                 a socket error
+ * @remark         Cares about ae->last_errno
  */
-
 int ajp_connection_tcp_get_message(ajp_endpoint_t * ae,
                                    jk_msg_buf_t *msg, jk_logger_t *l)
 {
@@ -964,9 +996,11 @@
     if (rc < 0) {
         ae->last_errno = errno;
         if (rc == JK_SOCKET_EOF) {
-            jk_log(l, JK_LOG_INFO,
-                   "(%s) Tomcat has forced a connection close for socket %d",
-                   ae->worker->name, ae->sd);
+            jk_log(l, JK_LOG_ERROR,
+                   "(%s) can't receive the response message from tomcat, "
+                   "tomcat (%s) has forced a connection close for socket %d",
+                   ae->worker->name, jk_dump_hinfo(&ae->worker->worker_inet_addr, buf),
+                   ae->sd);
         }
         else {
             jk_log(l, JK_LOG_ERROR,
@@ -1130,6 +1164,7 @@
         rdlen += this_time;
     }
 
+    JK_TRACE_EXIT(l);
     return (int)rdlen;
 }
 
@@ -1192,12 +1227,12 @@
  * send request to Tomcat via Ajp13
  * - first try to find reuseable socket
  * - if no one available, try to connect
- * - send request, but send must be see as asynchronous,
+ * - send request, but send must be seen as asynchronous,
  *   since send() call will return noerror about 95% of time
  *   Hopefully we'll get more information on next read.
  *
- * nb: reqmsg is the original request msg buffer
- *     repmsg is the reply msg buffer which could be scratched
+ * nb: op->request is the original request msg buffer
+ *     op->reply is the reply msg buffer which could be scratched
  *
  * Return values of ajp_send_request() function:
  * return value        op->recoverable   reason
@@ -1346,7 +1381,7 @@
 
     /*
      * From now on an error means that we have an internal server error
-     * or Tomcat crashed. In any case we cannot recover this.
+     * or Tomcat crashed.
      */
 
     if (JK_IS_DEBUG_LEVEL(l))
@@ -1994,7 +2029,7 @@
     s->secret = p->worker->secret;
 
     /*
-     * We get here initial request (in reqmsg)
+     * We get here initial request (in op->request)
      */
     if (!ajp_marshal_into_msgb(op->request, s, l, p)) {
         *is_error = JK_REQUEST_TOO_LARGE;
@@ -2015,8 +2050,8 @@
      */
     for (i = 0; i < p->worker->worker.retries; i++) {
         /*
-         * We're using reqmsg which hold initial request
-         * if Tomcat is stopped or restarted, we will pass reqmsg
+         * We're using op->request which hold initial request
+         * if Tomcat is stopped or restarted, we will pass op->request
          * to next valid tomcat.
          */
         err = ajp_send_request(e, s, l, p, op);



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


Re: svn commit: r604267 - /tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c

Posted by Rainer Jung <ra...@kippdata.de>.
Done (switching back).

Mladen Turk schrieb:
> rjung@apache.org wrote:
>> Author: rjung
>>  int ajp_connect_to_endpoint(ajp_endpoint_t * ae, jk_logger_t *l)
>>  {
>> @@ -841,7 +861,7 @@
>>  
>>      if (!IS_VALID_SOCKET(ae->sd)) {
>>          ae->last_errno = errno;
>> -        jk_log(l, JK_LOG_INFO,
>> +        jk_log(l, JK_LOG_ERROR,
>>                 "Failed opening socket to (%s) (errno=%d)",
>>                 jk_dump_hinfo(&ae->worker->worker_inet_addr, buf),
>> ae->last_errno);
> 
> IMO this will create duplicate errors lines in the log for the
> same cause. The parent function calling this one will log the error
> if this one fails. Now we'll have multiple of them.
> 
> I've changed the ERROR->INFO some time back just because of that.
> Is it really needed to have that changed to ERROR?
> It'll only confuse users thought.
> 
> Regards,
> Mladen

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


Re: svn commit: r604267 - /tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c

Posted by Mladen Turk <mt...@apache.org>.
rjung@apache.org wrote:
> Author: rjung
>  int ajp_connect_to_endpoint(ajp_endpoint_t * ae, jk_logger_t *l)
>  {
> @@ -841,7 +861,7 @@
>  
>      if (!IS_VALID_SOCKET(ae->sd)) {
>          ae->last_errno = errno;
> -        jk_log(l, JK_LOG_INFO,
> +        jk_log(l, JK_LOG_ERROR,
>                 "Failed opening socket to (%s) (errno=%d)",
>                 jk_dump_hinfo(&ae->worker->worker_inet_addr, buf), ae->last_errno);

IMO this will create duplicate errors lines in the log for the
same cause. The parent function calling this one will log the error
if this one fails. Now we'll have multiple of them.

I've changed the ERROR->INFO some time back just because of that.
Is it really needed to have that changed to ERROR?
It'll only confuse users thought.

Regards,
Mladen

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