401003 - Memory leak tests don't detect same ordinary failures as all.sh

Status: RESOLVED FIXED

(NSS :: Test, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

In bug 400841 comment 5, Slavo wrote:

> Memory leak checking doesn't check return values of NSS tools and, 
> if strsclnt fails it's not detected. Test is marked as FAILED only when 
> there is new leak found. 

He went on to say that if the same failures always occurred in both leak 
test runs and non-leak test runs, that would be OK, but now we're having 
test program failures that occur only in leak-test runs, and these failures
are going undetected.

So this bug calls the for the leak tests to detect all the same types of 
failure that the non-leak tests detect.  If a failure would turn a non-leak
test run orange (in tinderbox) then a leak test run should also turn orange
when it encounters that failure.

Comment 1

[Slavomir Katuscak]

Attachment: Patch.

This patch adds 3 things:

1. Functions run_commands_dbx and run_commands_valgrind returns return value which returns tested command.
2. Added checking of return values, failures are reported to both results.html and output.log (only failures).
3. Fixed issue from bug 400841 comment 2. I compared memory leak tests with basic ssl tests and there is always used nickname as parameter in strsclnt (-n TestUser). This was not used in memory leak tests before - it caused failures when client DB was in FIPS mode, otherwise it worked. Adding this parameter fixes the issue, please let me know if this is correct (FIPS feature) or it's a bug (should work also without parameter).

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 292590 [details] [diff] [review]
Patch.

I have one concern about this patch, in the following hunk.

>- echo "${DBX_CMD}" | ${DBX} ${COMMAND} 2>/dev/null | grep -v Reading 1>&2
>+	
>+ output=`echo "${DBX_CMD}" | ${DBX} ${COMMAND} 2>/dev/null | grep -v Reading`
>+ echo "$output" 1>&2
>+	
>+ echo "$output" | grep "exit code is 0"
>+ return $?

This puts the entire output of the dbx command into a shell variable 
named "output".  There's some limit to the size of the content of a 
shell variable, but I don't know what that limit it.  Is there any 
chance that the output of the dbx command will exceed that limit?

Comment 3

[Slavomir Katuscak]

I checked bash manpage and I didn't find any limit. 
Then I tested it on my laptop with 2GB RAM, with command q=`cat /dev/urandom` it was allocating memory until 2 gigs and than bash hanged (needed to use kill -9).

I think if we exceed limit like this, it means that we have much bigger problem with leaks than with limitation of bash.  

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I tested the limit on another POSIX shell, and found it was less than 32K.
Unless you have reason to believe that the output of DBX will never exceed
32K, I think this patch will have to use something other than a shell 
variable to hold the dbx output.

Comment 5

[Slavomir Katuscak]

Attachment: Patch v2.

Using temporary file instead for variable for output of dbx.
I don't think this is really needed, but 32K is a limit which we can exceed in some cases.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 295929 [details] [diff] [review]
Patch v2.

Please make one change, then r=nelson

>+	cat ${TMP_DBX} | grep "exit code is 0"

Change that line to:

        grep "exit code is 0" ${TMP_DBX} 

I've asked Julien for second review for branch.

Comment 7

[Slavomir Katuscak]

Trunk:
Checking in memleak.sh;
/cvsroot/mozilla/security/nss/tests/memleak/memleak.sh,v  <--  memleak.sh
new revision: 1.24; previous revision: 1.23
done

Comment 8

[Slavomir Katuscak]

Branch:
Checking in memleak.sh;
/cvsroot/mozilla/security/nss/tests/memleak/memleak.sh,v  <--  memleak.sh
new revision: 1.1.2.19; previous revision: 1.1.2.18
done