From: Mattias Jonsson
Date: October 1 2010 10:42pm
Subject: Re: bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3310)
 Bug#52002
List-Archive: http://lists.mysql.com/commits/119761
Message-Id: <4CA663D6.20707@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Tor,

Thank you for your patch.

I also use abort() in sql/sql_partition.cc, and would be like to switch 
to DBUG_SUICIDE() too.

Unfortunately when I tested DBUG_SUICIDE on my macbook, it succeeded 
with a command which should have crashed, an yet another run showed 
differences in the result file (probably due to there was code run in 
between of the kill() call and the actual death of the process).

I tested successfully with changing 'kill(getpid(), SIGKILL)' to 
'raise(SIGKILL), pause()'. The only worry I have with that solution is 
that according to the manual in mac os x, pause() is made obsolete by 
sigsuspend() but that function require more code.


So could you please:
1) change the 'kill(getpid(), SIGKILL)' to 'raise(SIGKILL), pause()' or 
something other that ensures that DBUG_SUICIDE() never returns (test 
also on mac os x).

2) include the following in your patch for bug#52002:
=== modified file 
'mysql-test/suite/parts/t/partition_debug_innodb-master.opt'
--- mysql-test/suite/parts/t/partition_debug_innodb-master.opt 
2010-08-13 07:50:25 +0000
+++ mysql-test/suite/parts/t/partition_debug_innodb-master.opt 
2010-10-01 20:12:51 +0000
@@ -1,1 +1,1 @@
---innodb-file-format-check --innodb-file-per-table=1 --skip-stack-trace 
--skip-core-file
+--innodb-file-format-check --innodb-file-per-table=1

=== removed file 
'mysql-test/suite/parts/t/partition_debug_myisam-master.opt'
--- mysql-test/suite/parts/t/partition_debug_myisam-master.opt 
2010-08-13 07:50:25 +0000
+++ mysql-test/suite/parts/t/partition_debug_myisam-master.opt 
1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
---skip-stack-trace --skip-core-file

=== modified file 'sql/sql_partition.cc'
--- sql/sql_partition.cc	2010-10-01 13:59:07 +0000
+++ sql/sql_partition.cc	2010-10-01 20:12:04 +0000
@@ -70,9 +70,8 @@
  #ifdef WITH_PARTITION_STORAGE_ENGINE
  #include "ha_partition.h"

-/* TODO: Change abort() to DBUG_SUICIDE() when bug#52002 is pushed */
  #define ERROR_INJECT_CRASH(code) \
-  DBUG_EVALUATE_IF(code, (abort(), 0), 0)
+  DBUG_EVALUATE_IF(code, (DBUG_SUICIDE(), 0), 0)
  #define ERROR_INJECT_ERROR(code) \
    DBUG_EVALUATE_IF(code, (my_error(ER_UNKNOWN_ERROR, MYF(0)), TRUE), 0)



And perhaps add some code to your test case to make sure it never 
executes anything after DBUG_SUICIDE().

Regards
Mattias

On 2010-10-01 11.07, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/next-mr-bf-binlogcore/ based on revid:luis.soares@stripped
>
>   3310 Tor Didriksen	2010-10-01
>        Bug#52002 binlog_index may cause incorrect failure report on failing test that follows it
>
>        For crash testing: kill the server without generating core file.
>       @ include/my_dbug.h
>          Use kill(getpid(), SIGKILL) which cannot be caught by signal handlers.
>          All DBUG_XXX macros should be no-ops in optimized mode, do that for DBUG_ABORT as well.
>       @ sql/binlog.cc
>          Kill server without generating core.
>       @ sql/handler.cc
>          Kill server without generating core.
>       @ unittest/gunit/CMakeLists.txt
>          Add unit test.
>       @ unittest/gunit/dbug-t.cc
>          Add unit test.
>
>      added:
>        unittest/gunit/dbug-t.cc
>      modified:
>        include/my_dbug.h
>        sql/binlog.cc
>        sql/handler.cc
>        unittest/gunit/CMakeLists.txt
> === modified file 'include/my_dbug.h'
> --- a/include/my_dbug.h	2010-07-15 11:16:06 +0000
> +++ b/include/my_dbug.h	2010-10-01 09:07:06 +0000

[...]

> @@ -111,6 +118,19 @@ extern  const char* _db_get_func_(void);
>   #define DBUG_CRASH_VOID_RETURN \
>     DBUG_CHECK_CRASH (_db_get_func_(), "_crash_return")
>
> +/*
> +  Make the program fail, without creating a core file.
> +  abort() will send SIGABRT which (most likely) generates core.
> +  Use SIGKILL instead, which cannot be caught.
> +  An alternative would be to use _exit(EXIT_FAILURE),
> +  but then valgrind would report lots of memory leaks.
> + */
> +#ifdef __WIN__
> +#define DBUG_SUICIDE() DBUG_ABORT()
> +#else
> +#define DBUG_SUICIDE() (_db_flush_(), kill(getpid(), SIGKILL))
> +#endif
> +
>   #else                                           /* No debugger */
>
>   #define DBUG_ENTER(a1)

[...]