This change is the result of a code audit. The changes are not drastic, but should...
authorSapan Bhatia <sapanb@cs.princeton.edu>
Sat, 28 Jun 2008 19:21:13 +0000 (19:21 +0000)
committerSapan Bhatia <sapanb@cs.princeton.edu>
Sat, 28 Jun 2008 19:21:13 +0000 (19:21 +0000)
Specifically:

* It should never *ever* stop running now (see the change in fdwatcher.ml)
* File descriptor masking is finer grained now

backend.ml
directfifowatcher.ml
dirwatcher.ml
fdwatcher.ml
frontend.ml
globals.ml
main.ml
tests/vsys_conctest.c

index 2f964ef..34dde6b 100644 (file)
@@ -15,10 +15,6 @@ open Fifowatcher
 open Frontend
 open Printf
 
-(** Helper functions:
-
-*)
-
 (** Turn an absolute path into a relative path. *)
 let delete_prefix prefix str =
   let len = String.length str in
@@ -35,7 +31,6 @@ let rec list_check lst elt =
     | [] -> false
     | car::cdr -> if (car==elt) then true else list_check cdr elt
 
-
 (** The backendhandler class: defines event handlers for events in
 the backend backend directory.
   @param dir_root The location of the backend in the server context (eg. root context for vservers)
@@ -45,15 +40,11 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
    let mk_rel_path = delete_prefix dir_root in object(this)
 
      (** Regular expression that defines a legal script name. Filter out
-       * temporary files using this *)
+       * temporary files using it *)
      val file_regexp = Str.regexp "^[a-zA-Z][a-zA-Z0-9_\.\-]*$"
      val acl_file_regexp = Str.regexp ".*acl$"
 
-     val dir_regexp = Str.regexp "^dir_";
-     val acl_regexp = Str.regexp ".*_.*";
-
      (** Somebody created a new directory *)
-     (* XXX Race condition here *)
      method private new_dir slice_list fqp func =
        let s = Unix.stat fqp in
          List.iter 
@@ -68,11 +59,11 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
            slice_list
 
      (** Somebody copied in a new script *)
-     (* XXX Race condition here *)
      method private new_script slice_list fqp =
        let s = Unix.stat fqp in
          List.iter (fun frontend->
-                      frontend#mkentry (mk_rel_path fqp) fqp (s.st_perm)) slice_list 
+                      frontend#mkentry (mk_rel_path fqp) fqp (s.st_perm)) 
+                   slice_list 
 
      method private make_filter acl_fqp =
        let filter = Hashtbl.create 16 in
@@ -110,7 +101,9 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
              let slice_list = 
                match acl_filter with
                  | None -> [] (* No ACL *) 
-                 | Some(filter) -> List.filter (fun fe->Hashtbl.mem filter (fe#get_slice_name ())) frontend_lst 
+                 | Some(filter) -> List.filter 
+                                     (fun fe->Hashtbl.mem filter (fe#get_slice_name ())) 
+                                     frontend_lst 
              in 
              let is_event = list_check evlist in
                if (is_event Create) then
@@ -122,13 +115,6 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
                    else
                      (* It's a new script *)
                      begin
-                       (*
-                        if (Str.string_match dir_regexp fname 0) then
-                        let fqp = String.concat "/" [dirname;String.sub fname 4 ((String.length fname)-4+1)]  in 
-                        let real_fqp = String.concat "/" [dirname;fname]  in 
-                        this#new_dir fqp this#handle_spool_event;
-                        Hashtbl.add spools fqp real_fqp
-                        else*)
                        this#new_script slice_list fqp
                      end
                  end
@@ -145,7 +131,7 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
                  end
            end
          else (* regex not matched *)
-           ()
+           fprintf logfd "Rejected weird entry %s\n" fname
 
      (** Initializer - build the initial tree based on the contents of /vsys *)
      initializer 
@@ -157,29 +143,31 @@ class backendHandler dir_root (frontend_lst: frontendHandler list) =
              let curfile = readdir dir_handle in
                if (not (this#is_acl curfile)) then
                  begin
-             let fqp = String.concat "/" [dir;curfile] in
-             let acl_fqp = String.concat "." [fqp;"acl"] in
-             let acl_filter = this#make_filter acl_fqp in
-             let slice_list = 
-               match acl_filter with
-                 | None -> [] (*frontend_lst -> No ACL => No Show *)
-                 | Some(filter) -> List.filter (fun fe->Hashtbl.mem filter (fe#get_slice_name ())) frontend_lst 
-             in
-               if (Str.string_match file_regexp curfile 0) then
-                 let s = Unix.stat fqp in
-                   begin
-                     match s.st_kind with
-                       | S_DIR ->
-                           this#new_dir slice_list fqp this#handle_dir_event;
-                           build_initial_tree fqp;
-                       | S_REG ->
-                           this#new_script slice_list fqp
-                       | _ ->
-                           fprintf logfd "Don't know what to do with %s\n" curfile;flush logfd
-                   end
+                   let fqp = String.concat "/" [dir;curfile] in
+                   let acl_fqp = String.concat "." [fqp;"acl"] in
+                   let acl_filter = this#make_filter acl_fqp in
+                   let slice_list = 
+                     match acl_filter with
+                       | None -> [] (*frontend_lst -> No ACL => No Show *)
+                       | Some(filter) -> List.filter 
+                                           (fun fe->Hashtbl.mem filter (fe#get_slice_name ())) 
+                                           frontend_lst 
+                   in
+                     if (Str.string_match file_regexp curfile 0) then
+                       let s = Unix.stat fqp in
+                         begin
+                           match s.st_kind with
+                             | S_DIR ->
+                                 this#new_dir slice_list fqp this#handle_dir_event;
+                                 build_initial_tree fqp;
+                             | S_REG ->
+                                 this#new_script slice_list fqp
+                             | _ ->
+                                 fprintf logfd "Don't know what to do with %s\n" curfile;flush logfd
+                         end
                  end
            with _
-               ->cont:=false;()
+           ->cont:=false;()
          done 
      in
        begin
index 60c0987..59b6127 100644 (file)
@@ -38,10 +38,13 @@ let openentry_int fifoin =
 
 
 (** Open fifos for a session. SHOULD NOt shutdown vsys if the fifos don't exist *)
-let openentry_in root_dir fqp_in backend_spec =
-    Dirwatcher.mask_watch root_dir;
+let openentry_in rp root_dir fqp_in backend_spec =
+  match rp with 
+    | 
+    | 
+    Dirwatcher.mask_watch root_dir fqp_in;
   let fd_in = openentry_int fqp_in in
-    Dirwatcher.unmask_watch root_dir [S_Open];
+    Dirwatcher.unmask_watch root_dir fqp_in;
   let (fqp,slice_name) = backend_spec in
     Hashtbl.replace direct_fifo_table fqp_in (Some(root_dir,fqp,slice_name,fd_in))
 
@@ -62,7 +65,8 @@ let connect_file fqp_in =
     Hashtbl.find direct_fifo_table fqp_in with _ -> None in
     match entry_info with
       | Some(_,execpath,slice_name,fifo_fdin) ->
-          fprintf logfd "Executing %s for slice %s\n" execpath slice_name;flush logfd;
+          (*fprintf logfd "Executing %s for slice %s\n" execpath
+           * slice_name;flush logfd;*)
           begin
             let len = String.length fqp_in in
             let fqp = String.sub fqp_in 0 (len-3) in
index e96c4e9..3122642 100644 (file)
@@ -29,27 +29,24 @@ let handle_dir_event dirname evlist str =
 let add_watch dir events handler =
   let evcheck = list_check events in
   let wd = Inotify.add_watch fd dir events in
-    Hashtbl.add masks dir (wd,handler);
     Hashtbl.add wdmap wd (dir,Some(handler))
-
       (* Ignore the possibility that the whole directory can disappear and come
        * back while it is masked *)
 
-let mask_watch dir =
+let mask_watch dir file =
   try 
-    let wd,_ = Hashtbl.find masks dir in
-      Inotify.rm_watch fd wd;
-      Hashtbl.remove wdmap wd
+    Hashtbl.replace masks (dir,file) true
   with _ ->
     ()
 
-let unmask_watch dir events =
-  let _,handler = try Hashtbl.find masks dir with Not_found->fprintf logfd "unmask called without mask: %s\n" dir;flush logfd;raise Not_found in
-    try 
-      Hashtbl.remove masks dir;
-      add_watch dir events handler
-    with Not_found -> ()
-
+let unmask_watch dir file =
+  if (Hashtbl.mem masks (dir,file)) then
+    begin
+      Hashtbl.remove masks (dir,file)
+    end
+  else
+    fprintf logfd "WARNING: %s,%s -- Unpaired unmask\n" dir file;flush logfd
+  
 let asciiz s =
   let rec findfirstnul str idx len =
     if ((idx==len) || 
@@ -73,7 +70,10 @@ let receive_event (eventdescriptor:fname_and_fd) (bla:fname_and_fd) =
                                in
                                    match handler with
                                      | None->fprintf logfd "Unhandled watch descriptor\n";flush logfd
-                                     | Some(handler)->handler wd dirname evlist purestr
+                                     | Some(handler)->
+                                         let mask_filter = Hashtbl.mem masks (dirname,purestr) in
+                                           if (not mask_filter) then
+                                                handler wd dirname evlist purestr
                        end
                    | _ -> ()) 
       evs
index 3453d54..95d4be4 100644 (file)
@@ -1,33 +1,35 @@
 (** Fdwatcher - The main event loop. Agnostic to the type of file descriptors
- involved.*)
 involved.*)
 
 open Printf
 open Globals
+open Printexc
 
 let fdset = ref []
 let cbtable = Hashtbl.create 1024
 
-(* The in descriptor is always open. Thanks to the broken semantics of
- * fifo outputs, the out descriptor must be opened a nouveau whenever we
- * want to send out data, and so we keep the associated filename as well.
- * Same with input fifos. Yipee.*)
 let add_fd (evpair:fname_and_fd) (fd_other:fname_and_fd) (callback:fname_and_fd->fname_and_fd->unit) = 
   let (fname,fd) = evpair in
-  fdset := (fd::!fdset);
-  Hashtbl.replace cbtable fd (callback,(evpair,fd_other))
+    fdset := (fd::!fdset);
+    Hashtbl.replace cbtable fd (callback,(evpair,fd_other))
 
 let del_fd fd =
   fdset:=List.filter (fun l->l<>fd) !fdset;
   flush logfd
 
 let start_watch () =
-    while (true)
-    do
-              let (fds,_,_) = try Unix.select !fdset [] [] (-1.) 
-              with e->
-                ([],[],[])
-              in
-                List.iter (fun elt->
-                             let (func,(evd,fd_other)) = Hashtbl.find cbtable elt in
-                               func evd fd_other) fds
-    done
+  while (true)
+  do
+    let (fds,_,_) = try Unix.select !fdset [] [] (-1.) 
+    with e->
+      ([],[],[])
+    in
+      List.iter (fun elt->
+                   let (func,(evd,fd_other)) = Hashtbl.find cbtable elt in
+                     try (* Never fail *)
+                       func evd fd_other
+                     with e->
+                       let wtf = Printexc.to_string e in
+                         fprintf logfd "%s\n" wtf
+                ) fds
+  done
index e8526cc..1c46b2c 100644 (file)
@@ -24,7 +24,9 @@ object(this)
         let res = Directfifowatcher.mkentry fqp abspath realperm slice_name in
           match res with 
             | Success ->
-                Directfifowatcher.openentry root_dir fqp (abspath,slice_name)
+                (* We don't want to get triggered when the .in descriptor is
+                 * opened *)
+                Directfifowatcher.openentry rp root_dir fqp (abspath,slice_name);
             | _ -> ()
 
   (** A new directory was created at the backend, make a corresponding directory
@@ -46,11 +48,7 @@ object(this)
               end;
         with Unix.Unix_error(_,_,_) ->
           Unix.mkdir fqp perm;
-        Directfifowatcher.add_dir_watch fqp
-
-
-
-            
+          Directfifowatcher.add_dir_watch fqp
 
   (** Functions corresponding to file deletion/directory removal *)
 
@@ -77,24 +75,24 @@ object(this)
           fprintf logfd "Hm. %s disappeared or not empty. Looks like slice %s shot itself in the foot\n" fqp (this#get_slice_name ());flush logfd
 
   initializer 
-          (
-        try 
-          let s = Unix.stat root_dir in
-            if (s.st_kind<>S_DIR) then
-              begin
-                Unix.unlink root_dir;
-                Unix.mkdir root_dir 0o700
-              end
-            else if (s.st_perm <> 0o700) then
-              begin
-                Unix.rmdir root_dir;
-                Unix.mkdir root_dir 0o700
-              end;
-        with Unix.Unix_error(_,_,_) ->
-          begin
+    (
+      try 
+        let s = Unix.stat root_dir in
+          if (s.st_kind<>S_DIR) then
+            begin
+              Unix.unlink root_dir;
+              Unix.mkdir root_dir 0o700
+            end
+          else if (s.st_perm <> 0o700) then
+            begin
+              Unix.rmdir root_dir;
+              Unix.mkdir root_dir 0o700
+            end;
+      with Unix.Unix_error(_,_,_) ->
+        begin
           try 
-          Unix.mkdir root_dir 0o700;
+            Unix.mkdir root_dir 0o700;
           with _ -> ();
-          end);
-        Directfifowatcher.add_dir_watch root_dir
+        end);
+          Directfifowatcher.add_dir_watch root_dir
 end
index 2dd7fe9..4c04057 100644 (file)
@@ -10,7 +10,6 @@ let failsafe = ref false
 
 let logfd = open_out_gen [Open_append;Open_creat] 0o644 !log_filepath
 type result = Success | Failed
-
 type fname_and_fd = string option * Unix.file_descr
 
 (* Relative path, never precededed by a '/' *)
diff --git a/main.ml b/main.ml
index ac7a8ce..0abccdb 100644 (file)
--- a/main.ml
+++ b/main.ml
@@ -16,44 +16,50 @@ let cmdspeclist =
     ("-daemon",Arg.Set(daemonize), "Daemonize");
     ("-conffile",Arg.Set_string(Globals.conffile), "Config file");
     ("-backend",Arg.Set_string(Globals.backend), "Backend directory");
-    ("-frontend",Arg.Tuple[Arg.String(fun s->cur_dir:=s);Arg.String(fun s->cur_slice:=s;input_file_list:=(!cur_dir,!cur_slice)::!input_file_list)], "frontendN,slicenameN");
+    ("-frontend",Arg.Tuple[Arg.String(fun s->cur_dir:=s);
+                           Arg.String(fun s->cur_slice:=s;
+                                             input_file_list:=(!cur_dir,!cur_slice)::!input_file_list)], 
+     "frontendN,slicenameN");
     ("-nochroot",Arg.Set(Globals.nochroot), "Run in non-chroot environment");
     ("-failsafe",Arg.Set(Globals.failsafe), "Never crash. Be stupid, but never crash. Use at your own risk.");
   ]
 
-let cont = ref true
-
 let _ =
-  printf "Vsys v%s\n" Globals.vsys_version;flush stdout;
+  printf "Starting Vsys v%s\n" Globals.vsys_version;flush stdout;
   fprintf logfd "Starting Vsys v%s\n" Globals.vsys_version;flush logfd;
   Arg.parse cmdspeclist (fun x->()) "Usage: vsys <list of mount points>";  
   if (!Globals.backend == "") then
-      printf "Try vsys --help\n"
+    printf "Try vsys --help\n"
   else
     begin
       if (!daemonize) then
         begin
           printf "Daemonizing\n";flush Pervasives.stdout;
-        let child = Unix.fork () in
-          if (child <> 0) then
-            begin
+          let child = Unix.fork () in
+            if (child <> 0) then
+              begin
                 let pidfile = open_out !Globals.pid_filepath in
                   fprintf pidfile "%d" child;
-                    close_out pidfile;
-                    exit(0)
-            end
-          end;
+                  close_out pidfile;
+                  exit(0)
+              end
+        end;
 
-            Dirwatcher.initialize ();
-            Directfifowatcher.initialize ();
+      Dirwatcher.initialize ();
+      Directfifowatcher.initialize ();
 
-            if (!Globals.conffile <> "") then
-              begin
-              let frontends = Conffile.read_frontends !Globals.conffile in
-                input_file_list:=List.concat [!input_file_list;frontends]
-              end;
+      if (!Globals.conffile <> "") then
+        begin
+          let frontends = Conffile.read_frontends !Globals.conffile in
+            input_file_list:=List.concat [!input_file_list;frontends]
+        end;
 
-            let felst = List.map (fun lst->let (x,y)=lst in fprintf logfd "Slice %s (%s)\n" x y;flush logfd;new frontendHandler lst) !input_file_list in
-                let _ = new backendHandler !Globals.backend felst in
-                 Fdwatcher.start_watch ()
+      let felst = List.map 
+                    (fun lst->let (x,y)=lst in 
+                       fprintf logfd "Slice %s (%s)\n" x y;
+                       flush logfd;
+                       new frontendHandler lst) 
+                    !input_file_list in
+      let _ = new backendHandler !Globals.backend felst in
+        Fdwatcher.start_watch ()
     end
index b1a17fa..e4b6913 100644 (file)
@@ -11,8 +11,8 @@ int main()
 {
   FILE *fp = NULL, *fp_in = NULL;
   FILE *out_fp = NULL, *diff_fp = NULL;
-  const char* topcmd = "/vsys/vtop.out";
-  const char* top_in_file = "/vsys/vtop.in";
+  const char* topcmd = "fe/test.out";
+  const char* top_in_file = "fe/test.in";
   char buf[4096];
   int fd_in = -1, fd_out;
   int res;
@@ -25,6 +25,7 @@ int main()
     int res;
     int nlines=0;
 
+    //usleep(200);
     printf("(%d)", count);fflush(stdout);
 
     if ((fd_out = open(topcmd, O_RDONLY | O_NONBLOCK)) < 0) {
@@ -32,10 +33,12 @@ int main()
       exit(-1);
     }
 
+    printf("Opening...\n");
     if ((fd_in = open(top_in_file, O_WRONLY)) < 0) {
       fprintf(stderr, "error opening %s\n", top_in_file);
       exit(-1);
     }
+    printf("Open.\n");
 
     if ((flag = fcntl(fd_out, F_GETFL)) == -1) {
       printf("fcntl get failed\n");
@@ -46,7 +49,10 @@ int main()
            FD_ZERO(&readSet);
            FD_SET(fd_out, &readSet);
 
+    printf("Selecting...\n");
+               sleep(1);
            res = select(fd_out + 1, &readSet, NULL, NULL, NULL);
+    printf("Selected...\n");
            if (res < 0) {
                    if (errno == EINTR || errno == EAGAIN) {
                            printf(".");