0005953: Unix.system does not handle EINTR

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

Project

Category

View Status

Date Submitted

Last Update

0005953

OCaml

OCaml otherlibs

public

2013-03-15 18:20

2013-03-22 12:44

Reporter

dim

Assigned To

dim

Priority

low

Severity

minor

Reproducibility

always

Status

resolved

Resolution

open

Platform

OS Version

Product Version

4.01.0+dev

Target Version

4.01.0+dev

Fixed in Version

Summary

0005953: Unix.system does not handle EINTR

Description

Right now Unix.system calls waitpid and does not handle EINTR. This means that if a signal handler is set somewhere else in the program a call to Unix.system may fail with this error.

I think Unix.system should waitpid again until it does not fail with EINTR since:

- this is what the system(3) function of the libc does
- the user does not have the pid of the sub-process so it cannot restart the call if it is interrupted

The attached patch replace the call to waitpid in Unix.system by one to waitpid_non_intr.

Steps To Reproduce

# #load "unix.cma";;
# Sys.set_signal Sys.sigchld (Sys.Signal_handle ignore);;
- : unit = ()
# let ic = Unix.open_process_in "sleep 5";;
val ic : in_channel = <abstr>
# Unix.system "sleep 10";;
Exception: Unix.Unix_error (Unix.EINTR, "waitpid", "").

Tags

No tags attached.

Attached Files

0001-handle-EINTR-in-Unix.system.patch [^] (1,321 bytes) 2013-03-15 18:20 [Show Content] [Hide Content]

From 3cccfc5ff56b882a890587aad1c2cab772b39fd6 Mon Sep 17 00:00:00 2001
From: Jeremie Dimino <jdimino@janestreet.com>
Date: Fri, 15 Mar 2013 16:58:46 +0000
Subject: [PATCH] handle EINTR in Unix.system

---
 otherlibs/unix/unix.ml |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/otherlibs/unix/unix.ml b/otherlibs/unix/unix.ml
index 8e573d8..8a69ca7 100644
--- a/otherlibs/unix/unix.ml
+++ b/otherlibs/unix/unix.ml
@@ -837,6 +837,10 @@ external setsid : unit -> int = "unix_setsid"
 
 (* High-level process management (system, popen) *)
 
+let rec waitpid_non_intr pid =
+  try waitpid [] pid
+  with Unix_error (EINTR, _, _) -> waitpid_non_intr pid
+
 let system cmd =
   match fork() with
      0 -> begin try
@@ -844,7 +848,7 @@ let system cmd =
           with _ ->
             exit 127
           end
-  | id -> snd(waitpid [] id)
+  | id -> snd(waitpid_non_intr id)
 
 let rec safe_dup fd =
   let new_fd = dup fd in
@@ -997,10 +1001,6 @@ let find_proc_id fun_name proc =
   with Not_found ->
     raise(Unix_error(EBADF, fun_name, ""))
 
-let rec waitpid_non_intr pid =
-  try waitpid [] pid
-  with Unix_error (EINTR, _, _) -> waitpid_non_intr pid
-
 let close_process_in inchan =
   let pid = find_proc_id "close_process_in" (Process_in inchan) in
   close_in inchan;
-- 
1.7.1

Relationships

Notes
(0008995) dim (developer) 2013-03-20 18:24	If nobody objects I'll merge the patch.

(0009001) dim (developer) 2013-03-22 12:44	Applied in revisions 13421 and 13422.

Issue History
Date Modified	Username	Field	Change
2013-03-15 18:20	dim	New Issue
2013-03-15 18:20	dim	Status	new => assigned
2013-03-15 18:20	dim	Assigned To	=> dim
2013-03-15 18:20	dim	File Added: 0001-handle-EINTR-in-Unix.system.patch
2013-03-15 18:27	dim	Description Updated	View Revisions
2013-03-20 18:24	dim	Note Added: 0008995
2013-03-22 12:44	dim	Note Added: 0009001
2013-03-22 12:44	dim	Status	assigned => resolved
2013-03-22 12:44	dim	Target Version	=> 4.01.0+dev