From acbdd1601fcfb161089d326e7e32b74b76f441e3 Mon Sep 17 00:00:00 2001 From: Sapan Bhatia Date: Sat, 28 Jun 2008 19:21:13 +0000 Subject: [PATCH] This change is the result of a code audit. The changes are not drastic, but should improve stability considerably. Specifically: * It should never *ever* stop running now (see the change in fdwatcher.ml) * File descriptor masking is finer grained now --- backend.ml | 72 ++++++++++++++++++------------------------- directfifowatcher.ml | 12 +++++--- dirwatcher.ml | 28 ++++++++--------- fdwatcher.ml | 36 ++++++++++++---------- frontend.ml | 46 +++++++++++++-------------- globals.ml | 1 - main.ml | 50 +++++++++++++++++------------- tests/vsys_conctest.c | 10 ++++-- 8 files changed, 129 insertions(+), 126 deletions(-) diff --git a/backend.ml b/backend.ml index 2f964ef..34dde6b 100644 --- a/backend.ml +++ b/backend.ml @@ -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 diff --git a/directfifowatcher.ml b/directfifowatcher.ml index 60c0987..59b6127 100644 --- a/directfifowatcher.ml +++ b/directfifowatcher.ml @@ -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 diff --git a/dirwatcher.ml b/dirwatcher.ml index e96c4e9..3122642 100644 --- a/dirwatcher.ml +++ b/dirwatcher.ml @@ -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 diff --git a/fdwatcher.ml b/fdwatcher.ml index 3453d54..95d4be4 100644 --- a/fdwatcher.ml +++ b/fdwatcher.ml @@ -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 diff --git a/frontend.ml b/frontend.ml index e8526cc..1c46b2c 100644 --- a/frontend.ml +++ b/frontend.ml @@ -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 diff --git a/globals.ml b/globals.ml index 2dd7fe9..4c04057 100644 --- a/globals.ml +++ b/globals.ml @@ -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 --- 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 "; 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 diff --git a/tests/vsys_conctest.c b/tests/vsys_conctest.c index b1a17fa..e4b6913 100644 --- a/tests/vsys_conctest.c +++ b/tests/vsys_conctest.c @@ -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("."); -- 2.43.0