changeset 1268:32bf673359d5

merge
author Jorge Arellano Cid <jcid@dillo.org>
date Wed, 05 Aug 2009 21:55:42 -0400
parents 0760652cf05e (current diff) 2590fc2b3e39 (diff)
children 3be98dfb084c
files doc/CCCwork.txt src/IO/http.c
diffstat 8 files changed, 111 insertions(+), 75 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Tue Aug 04 03:51:05 2009 +0000
+++ b/ChangeLog	Wed Aug 05 21:55:42 2009 -0400
@@ -12,6 +12,9 @@
  - Fixed CustProgressBox() for systems without weak symbols.
  - Handle signed chars. Added dIsspace() and dIsalnum() to dlib.
  - Added a_Dpip_get_attr_l() to DPIP's API.
+ - Changed the CCCs to build in one step (for both HTTP and DPI). This
+   is simpler and helps to avoid race conditions.
+ - Updated CCCwork.txt to the new scheme.
    Patches: Jorge Arellano Cid
 +- Fix segfault from AREA when MAP is missing name attribute.
  - Fix image map coordinates when margin/border/padding present.
--- a/doc/CCCwork.txt	Tue Aug 04 03:51:05 2009 +0000
+++ b/doc/CCCwork.txt	Wed Aug 05 21:55:42 2009 -0400
@@ -1,4 +1,4 @@
-Last review: August, 2009 --jcid
+Last review: August 04, 2009 --jcid
 
 
 ----------------------------
@@ -29,17 +29,27 @@
                                           .
                                           |
 
-*  currently the query chain isn't built at once because socket()
-needs the remote hostname's IP to return an associated FD. [1]
+*  a_Capi_open_url()  builds  both the Answer and Query chains at
+once  (Answer  first  then  Query), to ensure a uniform structure
+that avoids complexity (e.g. race conditions).
 
+*  Http_get()  sets  a  callback  for  the  DNS hostname resolve.
+Normally  it  comes  later, but may also by issued immediately if
+the hostname is cached.
 
-* the answer branch is built when capi 1F receives the FD.
+*  The  socket FD is passed by means of OpSend by the http module
+once the remote IP is known and the socket is connected.
+
 
 
 Function calls for HTTP CCC
 ---------------------------
 
   a_Capi_open_url
+    if (reload)
+      Capi OpStart 2B (answer)    [Capi] --> [dpi]  --> [IO]
+      Capi OpStart 1B (query)     [Capi] --> [http] --> [IO]
+        Http_get
     a_Cache_open_url
       if URL_E2EReload -> prepare reload
       if cached
@@ -49,22 +59,16 @@
         Cache_entry_add
         client enqueue
 
-    if (reload)
-      Capi OpStart 1B
-      OpStart, BCK:               [Capi] --> [http]
-        Http_get
-
     -//->
     a_Http_dns_cb
       Http_connect_socket
+        OpSend FD, BCK
+        OpSend FD, FWD
         Http_send_query
           a_Http_make_query_str
-          OpStart, BCK:           [Capi] --> [http] --> [IO]
-          OpSend, BCK
+            OpSend, BCK
               IO_submit
                 a_IOwatch_add_fd (DIO_WRITE, ...)
-          a_Chain_fcb(OpSend FD): this starts the receiving branch
-
 
 
   Note about 'web' structures. They're created using a_Web_new().
@@ -102,43 +106,36 @@
 
 CCC Construction:
 
-  a_Capi_open_url() calls a_Capi_dpi_send_cmd() when the URL
+  a_Capi_open_url()  calls  a_Capi_dpi_send_cmd()  when  the  URL
 belongs to a dpi and it is not cached.
 
-  a_Capi_dpi_send_cmd() creates the I-CCC backwards when requested
-to open a new connection. That is, Capi links Dpi, Dpi links IO, and
-Dpi sends the Socket FD to capi (Capi's OpSend 1F). If the connection
-exists, it is reused with OpSend.
-
-  When Capi receives the Socket FD, it creates the II-CCC backwards
-(Capi's OpStart 2B). Capi links Dpi, Dpi links IO, and Dpi starts IO
-(IO's OpStart 2B).
+  a_Capi_dpi_send_cmd()  builds  both the Answer and Query chains
+at  once (Answer first then Query), in the same way as HTTP does.
+Note  that  the  answer chain is the same for both, and the query
+chain only differs in the module in the middle ([http] or [dpi]).
 
 
+Function calls for DPI CCC
+--------------------------
+
+  a_Capi_open_url
+    a_Capi_dpi_send_cmd
+      Capi OpStart 2B (answer)    [Capi] --> [dpi]  --> [IO]
+      Capi OpStart 1B (query)     [Capi] --> [http] --> [IO]
+      a_Cache_open_url
+        [...]
 
 
 Normal termination:
 
   When  the dpi server is done, it closes the FD, and OpEnd flows
-from  IO  to  Capi (IO's OpEnd 2F). When in Capi, capi propagates
-OpEnd to I.
+from  IO  to  Capi (answer branch). When in Capi, capi propagates
+OpEnd to the query branch.
 
 Abnormal termination:
 
-  The  case  of  the transfer being aborted before the dpi server
-starts to send content exists, and it's handled like this:
-
-  When Capi receives "start_send_page" (Capi's OpSend 2F), it may
-detect  an  invalid  web  structure.  If  so, it issues an OpStop
-backwards (Dpi's OpStop 3B, not implemented).
+  The  transfer may be aborted by a_Capi_conn_abort_by_url(). The
+OpAbort is not yet standardized and has an ad-hoc implementation.
+One idea is to have OpAbort always propagate BCK and then FWD and
+to jump into the other chain when it gets to [Capi].
 
-  The  transfer  is  not currently aborted in II but just flagged
-ABORTED. It continues without further processing by Capi.
-
-
---
-
-[1]  The  CCC  construction  may be simplified by building all at
-once (i.e. not splitted with events). That could avoid complexity
-and allow for easier error handling.
-
--- a/src/IO/IO.c	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/IO/IO.c	Wed Aug 05 21:55:42 2009 -0400
@@ -59,13 +59,13 @@
 /* IO API  - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */
 
 /*
- * Return a newly created, and initialized, 'io' struct
+ * Return a new, initialized, 'io' struct
  */
-static IOData_t *IO_new(int op, int fd)
+static IOData_t *IO_new(int op)
 {
    IOData_t *io = dNew0(IOData_t, 1);
    io->Op = op;
-   io->FD = fd;
+   io->FD = -1;
    io->Flags = 0;
    io->Key = 0;
    io->Buf = dStr_sized_new(IOBufLen);
@@ -309,6 +309,11 @@
  */
 static void IO_submit(IOData_t *r_io)
 {
+   if (r_io->FD < 0) {
+      MSG_ERR("IO_submit: FD not initialized\n");
+      return;
+   }
+
    /* Insert this IO in ValidIOs */
    IO_ins(r_io);
 
@@ -346,14 +351,18 @@
          /* Write data using select */
          switch (Op) {
          case OpStart:
-            io = IO_new(IOWrite, *(int*)Data1); /* SockFD */
+            io = IO_new(IOWrite);
             Info->LocalKey = io;
             break;
          case OpSend:
             io = Info->LocalKey;
-            dbuf = Data1;
-            dStr_append_l(io->Buf, dbuf->Buf, dbuf->Size);
-            IO_submit(io);
+            if (Data2 && !strcmp(Data2, "FD")) {
+               io->FD = *(int*)Data1; /* SockFD */
+            } else {
+               dbuf = Data1;
+               dStr_append_l(io->Buf, dbuf->Buf, dbuf->Size);
+               IO_submit(io);
+            }
             break;
          case OpEnd:
          case OpAbort:
@@ -371,7 +380,7 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 1 FWD */
          /* Write-data status */
          switch (Op) {
          default:
@@ -385,10 +394,16 @@
          /* This part catches the reader's messages */
          switch (Op) {
          case OpStart:
-            io = IO_new(IORead, *(int*)Data2); /* SockFD */
+            io = IO_new(IORead);
             Info->LocalKey = io;
             io->Info = Info;
-            IO_submit(io);
+            break;
+         case OpSend:
+            io = Info->LocalKey;
+            if (Data2 && !strcmp(Data2, "FD")) {
+               io->FD = *(int*)Data1; /* SockFD */
+               IO_submit(io);
+            }
             break;
          case OpAbort:
             io = Info->LocalKey;
@@ -400,7 +415,7 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 2 FWD */
          /* Send read-data */
          io = Data1;
          switch (Op) {
--- a/src/IO/dpi.c	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/IO/dpi.c	Wed Aug 05 21:55:42 2009 -0400
@@ -615,13 +615,13 @@
                   *fd = SockFD;
                   Info->LocalKey = fd;
                   a_Chain_link_new(Info, a_Dpi_ccc, BCK, a_IO_ccc, 1, 1);
-                  a_Chain_bcb(OpStart, Info, Info->LocalKey, NULL);
-                  /* tell the capi to start the receiving branch */
-                  a_Chain_fcb(OpSend, Info, Info->LocalKey, "SockFD");
+                  a_Chain_bcb(OpStart, Info, NULL, NULL);
                }
             }
 
             if (st == 0 && SockFD != -1) {
+               a_Chain_bcb(OpSend, Info, &SockFD, "FD");
+               a_Chain_fcb(OpSend, Info, &SockFD, "FD");
                a_Chain_fcb(OpSend, Info, NULL, "DpidOK");
             } else {
                MSG_ERR("dpi.c: can't start dpi daemon\n");
@@ -645,7 +645,7 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 1 FWD */
          /* Send commands to dpi-server (status) */
          switch (Op) {
          case OpAbort:
@@ -675,7 +675,7 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* BCK */
+      } else {  /* 2 BCK */
          switch (Op) {
          case OpStart:
             conn = Dpi_conn_new(Info);
@@ -687,7 +687,12 @@
             }
 
             a_Chain_link_new(Info, a_Dpi_ccc, BCK, a_IO_ccc, 2, 2);
-            a_Chain_bcb(OpStart, Info, NULL, Data1); /* IORead, SockFD */
+            a_Chain_bcb(OpStart, Info, NULL, NULL); /* IORead */
+            break;
+         case OpSend:
+            if (Data2 && !strcmp(Data2, "FD")) {
+               a_Chain_bcb(OpSend, Info, Data1, Data2);
+            }
             break;
          case OpAbort:
             a_Chain_bcb(OpAbort, Info, NULL, NULL);
--- a/src/IO/http.c	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/IO/http.c	Wed Aug 05 21:55:42 2009 -0400
@@ -299,14 +299,9 @@
    _MSG_BW(S->web, 1, "Sending query to %s...", URL_HOST_(S->web->url));
 
    /* send query */
-   a_Chain_link_new(Info, a_Http_ccc, BCK, a_IO_ccc, 1, 1);
-   a_Chain_bcb(OpStart, Info, &S->SockFD, NULL);
    a_Chain_bcb(OpSend, Info, dbuf, NULL);
    dFree(dbuf);
    dStr_free(query, 1);
-
-   /* Tell the cache to start the receiving CCC for the answer */
-   a_Chain_fcb(OpSend, Info, &S->SockFD, "SockFD");
 }
 
 /*
@@ -379,6 +374,8 @@
          Http_socket_close(S);
          MSG("Http_connect_socket ERROR: %s\n", dStrerror(S->Err));
       } else {
+         a_Chain_bcb(OpSend, Info, &S->SockFD, "FD");
+         a_Chain_fcb(OpSend, Info, &S->SockFD, "FD");
          Http_send_query(S->Info, S);
          return 0; /* Success */
       }
@@ -476,6 +473,7 @@
    S = a_Klist_get_data(ValidSocks, SKey);
    if (S) {
       if (!a_Web_valid(S->web)) {
+         a_Chain_bcb(OpAbort, S->Info, NULL, NULL);
          a_Chain_fcb(OpAbort, S->Info, NULL, NULL);
          dFree(S->Info);
          Http_socket_free(SKey);
@@ -486,6 +484,7 @@
          /* start connecting the socket */
          if (Http_connect_socket(S->Info) < 0) {
             MSG_BW(S->web, 1, "ERROR: %s", dStrerror(S->Err));
+            a_Chain_bcb(OpAbort, S->Info, NULL, NULL);
             a_Chain_fcb(OpAbort, S->Info, NULL, NULL);
             dFree(S->Info);
             Http_socket_free(SKey);
@@ -495,6 +494,7 @@
          /* DNS wasn't able to resolve the hostname */
          MSG_BW(S->web, 0, "ERROR: Dns can't resolve %s",
             (S->use_proxy) ? URL_HOST_(HTTP_Proxy) : URL_HOST_(S->web->url));
+         a_Chain_bcb(OpAbort, S->Info, NULL, NULL);
          a_Chain_fcb(OpAbort, S->Info, NULL, NULL);
          dFree(S->Info);
          Http_socket_free(SKey);
@@ -560,6 +560,10 @@
             /* ( Data1 = Web ) */
             SKey = Http_sock_new();
             Info->LocalKey = INT2VOIDP(SKey);
+            /* link IO */
+            a_Chain_link_new(Info, a_Http_ccc, BCK, a_IO_ccc, 1, 1);
+            a_Chain_bcb(OpStart, Info, NULL, NULL);
+            /* async. connection */
             Http_get(Info, Data1);
             break;
          case OpEnd:
@@ -575,7 +579,7 @@
             dFree(Info);
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 1 FWD */
          /* HTTP send-query status branch */
          switch (Op) {
          default:
--- a/src/IO/iowatch.cc	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/IO/iowatch.cc	Wed Aug 05 21:55:42 2009 -0400
@@ -22,7 +22,8 @@
 //
 void a_IOwatch_add_fd(int fd, int when, FileHandler Callback, void *usr_data=0)
 {
-   add_fd(fd, when, Callback, usr_data);
+   if (fd >= 0)
+      add_fd(fd, when, Callback, usr_data);
 }
 
 //
@@ -30,6 +31,7 @@
 //
 void a_IOwatch_remove_fd(int fd, int when)
 {
-   remove_fd(fd, when);
+   if (fd >= 0)
+      remove_fd(fd, when);
 }
 
--- a/src/capi.c	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/capi.c	Wed Aug 05 21:55:42 2009 -0400
@@ -367,6 +367,9 @@
          a_Capi_conn_abort_by_url(web->url);
          /* create a new connection and start the CCC operations */
          conn = Capi_conn_new(web->url, web->bw, "http", "none");
+         /* start the reception branch before the query one because the DNS
+          * may callback immediatly. This may avoid a race condition. */
+         a_Capi_ccc(OpStart, 2, BCK, a_Chain_new(), conn, "http");
          a_Capi_ccc(OpStart, 1, BCK, a_Chain_new(), conn, web);
       }
       use_cache = 1;
@@ -475,6 +478,7 @@
       /* Create a new connection data struct and add it to the list */
       conn = Capi_conn_new(url, bw, server, cmd);
       /* start the CCC operations */
+      a_Capi_ccc(OpStart, 2, BCK, a_Chain_new(), conn, server);
       a_Capi_ccc(OpStart, 1, BCK, a_Chain_new(), conn, server);
 
    } else {
@@ -557,18 +561,17 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 1 FWD */
          /* Command sending branch (status) */
          switch (Op) {
          case OpSend:
             if (!Data2) {
                MSG_WARN("Capi.c: Opsend [1F] Data2 = NULL\n");
-            } else if (strcmp(Data2, "SockFD") == 0) {
-               /* start the receiving branch */
-               capi_conn_t *conn = Info->LocalKey;
+            } else if (strcmp(Data2, "FD") == 0) {
+               conn = Info->LocalKey;
                conn->SockFD = *(int*)Data1;
-               a_Capi_ccc(OpStart, 2, BCK, a_Chain_new(), Info->LocalKey,
-                          conn->server);
+               /* communicate the FD through the answer branch */
+               a_Capi_ccc(OpSend, 2, BCK, conn->InfoRecv, &conn->SockFD, "FD");
             } else if (strcmp(Data2, "DpidOK") == 0) {
                /* resume pending dpi requests */
                Capi_conn_resume();
@@ -596,16 +599,22 @@
 
    } else if (Branch == 2) {
       if (Dir == BCK) {
-         /* Server listening branch (status)
-          * (Data1 = conn; Data2 = {"HttpFD" | "DpiFD"}) */
+         /* Answer branch */
          switch (Op) {
          case OpStart:
+            /* Data1 = conn; Data2 = {"http" | "<dpi server name>"} */
             conn = Data1;
             Capi_conn_ref(conn);
             Info->LocalKey = conn;
             conn->InfoRecv = Info;
             a_Chain_link_new(Info, a_Capi_ccc, BCK, a_Dpi_ccc, 2, 2);
-            a_Chain_bcb(OpStart, Info, &conn->SockFD, Data2);
+            a_Chain_bcb(OpStart, Info, NULL, Data2);
+            break;
+         case OpSend:
+            /* Data1 = FD */
+            if (Data2 && strcmp(Data2, "FD") == 0) {
+               a_Chain_bcb(OpSend, Info, Data1, Data2);
+            }
             break;
          case OpAbort:
             conn = Info->LocalKey;
@@ -618,7 +627,7 @@
             MSG_WARN("Unused CCC\n");
             break;
          }
-      } else {  /* FWD */
+      } else {  /* 2 FWD */
          /* Server listening branch */
          switch (Op) {
          case OpSend:
--- a/src/web.cc	Tue Aug 04 03:51:05 2009 +0000
+++ b/src/web.cc	Wed Aug 05 21:55:42 2009 -0400
@@ -139,6 +139,7 @@
    a_Image_unref(web->Image);
    dFree(web->filename);
    dList_remove(ValidWebs, (void *)web);
+   _MSG("a_Web_free: ValidWebs=%d\n", dList_length(ValidWebs));
    dFree(web);
 }