Hanging sh.exe process

[TheWitness]

Does the following accomplish the same?

TheWitness

[BelgianViking]

Just wanted to share that I am running some scripts (fping for calcuting delay and loss), and some hosts are not reachable all the time. This might be the reason for some processes waiting long before they eventually time out, and the reason for these regular hanging SH.EXE processes.
I wonder if it would not be feasable (I would almost say 'easy', but for that I consider myself too novice in the field) to stop running data gatherings for devices that have been put in the 'down' state by the health check.

If a quick patch could possibly solve the problem, I will be glad to help testing it out.

[TheWitness]

Mr. Viking,

Please apply the patch of lib/ping.php in the following location and let me know if fping is better behaved.

TheWitness

http://svn.cacti.net/cgi-bin/viewcvs.cg ... 7&view=log

It should work better. In addition, you should apply the modified ss_fping.php at the following location:

http://svn.cacti.net/cgi-bin/viewcvs.cg ... 5&view=log

[ErikF]

As requested:

Code: Select all

...
    /* Disable thread cancellation from this point forward. */
    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cancel_state);
    
    if ((cur = malloc(sizeof(struct pid))) == NULL) {
        (void)close(pdes[0]);
        (void)close(pdes[1]);
        pthread_setcancelstate(cancel_state, NULL);
        return -1;
    }

+    /* Split command line into 'dargv' */
+    char** dargv = NULL;
+    int dargc = 0;
+    char* cmd = strdup(command);
+    char* pc;
+    char* s = cmd;
+    while ((pc = strtok(s, " ")) != NULL)
+    {
+        dargv = realloc(dargv, sizeof(char*) * (dargc + 1));
+        dargv[dargc++] = pc;
+        s = NULL;
+    }
+    dargv = realloc(dargv, sizeof(char*) * (dargc + 1));
+    dargv[dargc++] = NULL;

    /* Lock the list mutex prior to forking, to ensure that
     * the child process sees PidList in a consistent list state.
     */
    pthread_mutex_lock(&ListMutex);
...

Code: Select all

...
        /* Close all the other pipes in the child process.
         * Posix.2 requires this, tho I don't know why.
         */
        for (p = PidList; p; p = p->next)
            (void)close(p->fd);

        /* Execute the command. */
!        execve(dargv[0], dargv, environ);
        _exit(127);
        /* NOTREACHED */
    }

+    free(dargv);
+    free(cmd);
...

The "kill(cur->pid, SIGKILL);" which you added to "nft_pclose" should help if the process though hangs, but can not resolve the problem if the started process just hung without writing anything into the pipe (which is the scenario I am dealing with).

[ErikF]

I applied the "kill" patch to original sources (which do not contain my patch) and tested it. Unfortunately it does not help. Although the kill function returns 'success', the process does not get killed and it stalls in "waitpid".

Additionally, even if I first said that "kill" should help in each case, after looking closer at it, I don't think it's that good idea. Because the "kill" is currently applied for all started child processes regardless whether they are still running or not, this could cause other troubles. I think it would be more safe to invoke the "kill" only if really needed.

BTW, In file "util.c", function "cacti_log", there is a potential buffer exploit.

Code: Select all

if (set.verbose >= POLLER_VERBOSITY_NONE) {
	snprintf(flogmessage, LOGSIZE-1, "CACTID: %s", logmessage);
	printf(flogmessage);
}

The "printf" should be changed to "printf("%s", flogmessage);" to prevent problems with '%s' sequences in 'flogmessage'.

[TheWitness]

ErikF,

Thanks for your assistance. I have made both changes. I was wondering if you could look at a function that has me perplexed in that certain instances (Linux variants) are having difficulty with the implementation.

Look at the beta/svn version of poller.c lines 570 and 571. As I mentioned, some versions of Linux are puking at the method I am handling strings. Your thoughts/peer review would be welcomed.

TheWitness

[BelgianViking]

Witness,

I've updated ping.php and ss_fping.php, but I still get a hanging SH.EXE from time to time.

[TheWitness]

Did you update nft_popen.c from the CACTID SVN and remake Cactid?

Larry

[ErikF]

Look at the beta/svn version of poller.c lines 570 and 571. As I mentioned, some versions of Linux are puking at the method I am handling strings. Your thoughts/peer review would be welcomed.

I can't find any problem with the "snprintf". If you ask me, I'd say it's correct this way.

However, there is a small bug with the "strip_quotes" function. At least it does not do what the comment of that particular function is announcing ("...remove beginning and ending quotes from a string..."). It removes only a beginning quote or backslash but not the ending.

Additionally, "strip_quotes" and "strip_alpha" both will crash (and that indeed is OS and/or compiler dependant) if the string which is passed to them is allocated in a read-only segment. But, this does not apply for the instances I have seen in "poller.c" file.

So, the current implementations should work.

[TheWitness]

Thanks for the feedback!!

Larry

[adrianmarsh]

Wow.. been a while since I read this thread.... TheWitness: Has there been some fix I should apply?

Anyway, I've come up with a bodge-job (not a good one, but then what workaround ever is).

Basically, I've improved slightly on the "kill" script thats discussed on page1. I'm posting it here in case anyone wants to use. Its really basic, but works for me. Use at own risk though, as depending on your environment and usage it could kill processes you don't want it to. Seriously - understand the script before you use it...

Heres the steps:

1) Install a basic cygwin (+pdksh) into c:\cygwin (or modify the script to match)
2) Install sysinternals.com pstools (into c:\program files\sysinternals)
3) Put this script in c:\cygwin\bin (or somewhere)

Script takes two parameters. 1st is the process is kill, as pslist.exe will see it (without a file extension), and the 2nd is a process to ignore (again no extension). If a process is found, then the script will log its kill decision to c:\cygwin\tmp\result.txt. No process found = no log entry.

Setup a scheduled task. exe should be : c:\cygwin\bin\pdksh.exe sh shstat run in : c:\cygwin\bin, after your tested it manually, set it for a regular run (up to you, but I did 5 mins). Edit the script to change the time check, I set it to 6m (360s) to catch any post-cacti leftover.

I used sysinternals pslist.exe because I couldn't get cygwins ps.exe to return process time info. I added the "exclude" option because pslist.exe ignores file extensions, and so on a system with mcafee installed (which has a process called shstat.exe) would find both "sh" and "shstat". Not perfect I know. I've had it running on my system for 24 hours, and it seems ok, but its not found a process to kill yet, so I've really to see if it works.

A.