[PATCH] zsystem flock: zclose descriptor after unsuccessful lock

[PATCH] zsystem flock: zclose descriptor after unsuccessful lock

[Sebastian Gniazdowski]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

Hello
I have background scripts using zsystem flock. After a few hours OS X is out of descriptors. Turns out system.c doesn't close descriptor initially opened and passed to movefd(), when lock doesn't succeed. The patch adds preceding zclose(flock_fd) to all return 1 and return 2 code paths.

Just to mention, I might still resolve the script issues by changing code to use subshell closed on unsuccessful lock, unsure yet.

-- 
Sebastian Gniazdowski
psprint /at/ zdharma.org

[-- Attachment #2: zsystem_zclose.diff.txt --]
[-- Type: text/plain, Size: 1139 bytes --]

diff --git a/Src/Modules/system.c b/Src/Modules/system.c
index 3eecd7e..9fd4d25 100644
--- a/Src/Modules/system.c
+++ b/Src/Modules/system.c
@@ -649,22 +649,30 @@ bin_zsystem_flock(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
     if (timeout > 0) {
 	time_t end = time(NULL) + (time_t)timeout;
 	while (fcntl(flock_fd, F_SETLK, &lck) < 0) {
-	    if (errflag)
+	    if (errflag) {
+                zclose(flock_fd);
 		return 1;
+            }
 	    if (errno != EINTR && errno != EACCES && errno != EAGAIN) {
+                zclose(flock_fd);
 		zwarnnam(nam, "failed to lock file %s: %e", args[0], errno);
 		return 1;
 	    }
-	    if (time(NULL) >= end)
+	    if (time(NULL) >= end) {
+                zclose(flock_fd);
 		return 2;
+            }
 	    sleep(1);
 	}
     } else {
 	while (fcntl(flock_fd, timeout == 0 ? F_SETLK : F_SETLKW, &lck) < 0) {
-	    if (errflag)
+	    if (errflag) {
+                zclose(flock_fd);
 		return 1;
+            }
 	    if (errno == EINTR)
 		continue;
+            zclose(flock_fd);
 	    zwarnnam(nam, "failed to lock file %s: %e", args[0], errno);
 	    return 1;
 	}

Re: [PATCH] zsystem flock: zclose descriptor after unsuccessful lock

[Bart Schaefer]

On Sat, Dec 30, 2017 at 7:32 AM, Sebastian Gniazdowski
<psprint@zdharma.org> wrote:
> The patch adds preceding zclose(flock_fd) to all return 1 and return 2 code paths.

Nothing wrong with the patch but the original code structure looks
funny.  Why do we return without calling zwarnnam() when errflag?  I
suppose the assumption is that it only gets set if we already output
some other error message?

Re: [PATCH] zsystem flock: zclose descriptor after unsuccessful lock

[Peter Stephenson]

Not very online at the moment so please excuse format but I think you'll find there's somewhere a test so we don't output messages when errflag is already set (but it could be in some other code path than the one you're talking about).

pws

On 30 December 2017 17:44:28 GMT+00:00, Bart Schaefer <schaefer@brasslantern.com> wrote:
>On Sat, Dec 30, 2017 at 7:32 AM, Sebastian Gniazdowski
><psprint@zdharma.org> wrote:
>> The patch adds preceding zclose(flock_fd) to all return 1 and return
>2 code paths.
>
>Nothing wrong with the patch but the original code structure looks
>funny.  Why do we return without calling zwarnnam() when errflag?  I
>suppose the assumption is that it only gets set if we already output
>some other error message?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.