madscientist / msjnc

MadScientist Juniper Network Connect Session Manager
MIT License
69 stars 20 forks source link

Restarting msjnc with a connected session can't find the pid #13

Open wangmaster opened 10 years ago

wangmaster commented 10 years ago

Here's the log snippet:

2013-09-17 09:34:13-0500: MadScientist JNC Session Manager 2.5 (17 Feb 2013)
2013-09-17 09:34:13-0500: Session: Change from Disconnected to Connected
2013-09-17 09:34:13-0500: Session: Log says Connected, but no PID: disconnecting.

Here's where it's failing:

sub _getpid {
    my $arg = $_[0] ? "-p '$_[0]'" : '-e';
    # This appears to be the most portable option.
    # Kind of gross that we may have to run ps(1) on every update, but...
    local $_ = `ps $arg -o pid= -o comm=`;
    return m,^\s*(\d+)\s(?:.*/)?ncsvc$, ? $1 : 0;
}

I don't understand why it's failing though. If I move the begin/end line (^ $) on the return regex everything works fine.

Fedora 19: perl --version: This is perl 5, version 16, subversion 3 (v5.16.3) built for x86_64-linux-thread-multi ps --version procps-ng version 3.3.8

In case it's relevant. But as far as I can tell the ps output does have begin and end of line.

madscientist commented 10 years ago

Likely your version of ps has slightly different output. Can you run the command in question (ps e -o pid= -o comm=) and paste the ncsvc line into the issue? Please cut and paste it exactly because even a single character can make all the difference when dealing with regexs.

wangmaster commented 10 years ago

I wrote this simple perl script (did the same perl thing you did just to make sure it's an apples-to-apples comparison)

!/usr/bin/perl

$_=ps -e -o pid= -o comm=; print;

and redirected the output to >ps.out

Here's a cut and paste of the lines around it.

14664 msjnc 14680 ncsvc 14702 nxproxy

Here's a hexdump -C of it: 000011f0 6b 65 72 2f 32 3a 32 0a 31 34 36 36 34 20 6d 73 |ker/2:2.14664 ms| 00001200 6a 6e 63 0a 31 34 36 38 30 20 6e 63 73 76 63 0a |jnc.14680 ncsvc.| 00001210 31 34 37 30 32 20 6e 78 70 72 6f 78 79 0a 31 34 |14702 nxproxy.14|

So it looks like the proper EOL (0x0A) exists.

wangmaster commented 10 years ago

In case it helps, I have a debian wheezy vm and an ubuntu 13.04 vm and modified the script do this:

!/usr/bin/perl

$=ps -e -o pid= -o comm=; print m,^\s(\d+)\s(?:._/)?init$,;

And that fails as well with the ^ $'s in place. Remove them and they properly print PID 1.

madscientist commented 10 years ago

It's not that you're missing any newlines. I think it's because it's returning multiple lines. I'm not sure why this behavior is different for you. Please try changing the return line to be this:

return m,^\s*(\d+)\s(?:.*/)?ncsvc$,m ? $1 : 0;

and see if that works.

wangmaster commented 10 years ago

That worked. I won't pretend to know perl well enough to have any idea what that means though. But this is cute: $ ps -e -o pid= -o comm= | grep -P '^\s(\d+)\s(?:./)?ncsvc$' 14680 ncsvc

Curious why this only fails within perl.

madscientist commented 10 years ago

Since you said you're curious, I'll elucidate (... pontificate? :-)) : the difference in the two is that in grep it's chopping the input up line by line, and checking the regex against each line individually. In the Perl example it's taking the output and putting it into a single string, with embedded newlines, and checking the regex against the entire string. So in the Perl case ^ is matching the beginning of the string and $ is matching the end of the string.

Adding the m to the end of the regex tells Perl regex parser that it should change the meaning of ^ and $ to match beginning/end of line instead of beginning/end of string.

wangmaster commented 10 years ago

Ahh.. Thanks for that explanation. That makes sense. I was assuming that was what was already happening (bad assumption).

Thanks again for the quick fix and explaining it as well. This thing is working perfectly now. Thanks for providing this awesome tool :)

aperomsik commented 10 years ago

Suggested fix works for me too.

simonsigre commented 8 years ago

The fix also worked for me>

sub getpid { my $arg = $[0] ? "-p '$_[0]'" : '-e';

This appears to be the most portable option.

# Kind of gross that we may have to run ps(1) on every update, but...
local $_ = `ps $arg -o pid= -o comm=`;
return m,^\s*(\d+)\s(?:.*/)?ncsvc$,m ? $1 : 0;

}