MySQL Bugs: #85825: re-reading changed MTR source files is not safe in mysqltest

Bug #85825	re-reading changed MTR source files is not safe in mysqltest
Submitted:	6 Apr 2017 8:01	Modified:	2 May 2017 8:51
Reporter:	Laurynas Biveinis (OCA)	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: Tests	Severity:	S7 (Test Cases)
Version:	5.6+	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	mtr, mysql test framework, mysqltest

Description:
Say one implements a wait-for-condition loop in MTR, where a Perl snippet is responsible for checking that condition and returning a result by writing an MTR variable assignment to a temp MTR file, which is then sourced and deleted by the testcase.

This will not work, because mysqltest will read that temp file only on the first loop iteration. On any subsequent iterations it will not reread the file and will reuse the temp file contents from the first iteration, silently.

This happens to work in a straight line code, probably due to quirks in the way mysqltest considers the source file to be already read-in.

How to repeat:
let $counter = 2;

while ($counter)
{
  --exec echo --let \$foo = $counter > $MYSQL_TMP_DIR/foo.inc
  cat_file $MYSQL_TMP_DIR/foo.inc;
  source $MYSQL_TMP_DIR/foo.inc;
  remove_file $MYSQL_TMP_DIR/foo.inc;
  echo foo = $foo;
  dec $counter;
}

gives the following output:

--let $foo = 2
foo = 2
--let $foo = 1
foo = 2

The second loop iteration failed to get the desired foo value even though it is in the source file.

It happens to work with loop unrolled:

write_file $MYSQL_TMP_DIR/foo.inc;
let $foo = 1;
EOF
cat_file $MYSQL_TMP_DIR/foo.inc;
source $MYSQL_TMP_DIR/foo.inc;
remove_file $MYSQL_TMP_DIR/foo.inc;
echo foo = $foo;

write_file $MYSQL_TMP_DIR/foo.inc;
let $foo = 2;
EOF
cat_file $MYSQL_TMP_DIR/foo.inc;
source $MYSQL_TMP_DIR/foo.inc;
remove_file $MYSQL_TMP_DIR/foo.inc;
echo foo = $foo;

producing output

let $foo = 1;
foo = 1
let $foo = 2;
foo = 2

Suggested fix:
Decide whether mysqltest should support re-reading changed MTR source files correctly or not. Fix or document according to that.

Hello Laurynas,

Thank you for the report.

Thanks,
Umesh

This looks like not an issue with 'source' command, I was able reproduce the
issue with following test case.

Test case:
----------
let $counter = 2;

while ($counter)
{
  --exec echo "let \$foo = $counter;" > $MYSQL_TMP_DIR/foo.inc
  --source $MYSQL_TMP_DIR/foo.inc
  echo foo = $foo;
  dec $counter;
}

Output:
-------
foo = 2
foo = 2
main.temp                                [ pass ]      2

In the first iteration, mysqltest reads the test cases from a test file and
stores them in a Prealloced_array object.

Content of pre-allocated array after the first iteration:
---------------------------------------------------------
let $counter = 2

while ($counter)
{
exec echo "let \$foo = $counter;" > $MYSQL_TMP_DIR/foo.inc
source $MYSQL_TMP_DIR/foo.inc
let $foo = 2   =======> $foo = 2, right operand is a value
echo foo = $foo
dec $counter
}

During the second iteration, instead of reading the test cases from the test
file, mysqltest reads them from pre-allocated array. Because of this 'let
$foo = 2' gets executed again and hence the output value is 2.

This can be fixed by modifying the test, in the exec command, "$counter"
should be escaped i.e "\$counter".

Modified test:
--------------
let $counter = 2;

while ($counter)
{
  --exec echo "let \$foo = \$counter;" > $MYSQL_TMP_DIR/foo.inc
  --source $MYSQL_TMP_DIR/foo.inc
  echo foo = $foo;
  dec $counter;
}

Contents of pre-allocated array:
--------------------------------
let $counter = 2

while ($counter)
{
exec echo "let \$foo = \$counter;" > $MYSQL_TMP_DIR/foo.inc
source $MYSQL_TMP_DIR/foo.inc
let $foo = $counter       ====> Note: Here the right operand is not a
hard-coded value, instead its a variable which gets parsed to the
                                correct value in do_let() method
echo foo = $foo
dec $counter
}

Output:
-------
bash > ./mtr --mem main.temp
...
...
...
==============================================================================

TEST                                      RESULT   TIME (ms) or COMMENT
--------------------------------------------------------------------------

worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 13000..13009
foo = 2
foo = 1
main.temp                                [ pass ]      2
--------------------------------------------------------------------------
The servers were restarted 0 times
Spent 0.002 of 14 seconds executing testcases

Completed: All 1 tests were successful.

I see the bug has been set to "Need Feedback" - what feedback should I provide?

If it is about the proposed fix of quoting $counter to \$counter, then I believe it is not a fix here, but only a very specific workaround for this particular example. The workaround exploits the fact that the $counter is updated correctly between the loop iterations and so makes the sourced MTR file immutable between them. This workaround is not always accessible.

I encountered another similar scenario for which, unfortunately, the suggested workaround is not applicable.

--let $restart_counter = 0

while($restart_counter < 2)
{
  --echo *** Iteration $restart_counter ***
  # Determining server PID file name.
  --let $pid_file = `SELECT @@global.pid_file`
  --echo $pid_file

  # Extracting mysqld PID from $pid_file to an MTR variable $pid_no.
  --let $pid_no_include_file = $MYSQL_TMP_DIR/pid_no_include_file.inc$restart_counter
  --exec echo --let \$pid_no = `cat $pid_file` > $pid_no_include_file
  --echo $pid_no_include_file
  --cat_file $pid_no_include_file
  --source $pid_no_include_file
  --remove_file $pid_no_include_file
  --echo $pid_no

  --source include/restart_mysqld.inc

  --inc $restart_counter
}
******************************************************************
For the 'Iteration 1' the result is not expected.

*** Iteration 0 ***
<build_dir>/mysql-test/var/run/mysqld.1.pid
<build_dir>/mysql-test/var/tmp/pid_no_include_file.inc0
--let $pid_no = 9343
9343
# restart
*** Iteration 1 ***
<build_dir>/mysql-test/var/run/mysqld.1.pid
<build_dir>/mysql-test/var/tmp/pid_no_include_file.inc1
--let $pid_no = 9379
9343
# restart

******************************************************************

Here, I am even using different temporary include file names on each iteration, but result is still invalid.