Merge lp:~sergei.glushchenko/percona-xtrabackup/2.2-xb-bug1395143 into lp:percona-xtrabackup/2.2

[Sergei Glushchenko]

    Following leaks fixed:

    - Memory allocated for pmap_cur in page_map_file_open was never
      released.
    - Memory allocated for ivbuffer in xb_crypt_read_open was never
      released.
    - Doublewrite buffer allocated in open_or_create_data_files was
      not released after xtrabackup applied incremental deltas.
    - dst_log_file was never released.
    - fil_system was never released.
    - Memory allocated in innodb_init_param was not allocated after
      apply-log stage.

http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-param/259/

[Alexey Kopytov (akopytov)]

Sergei,

I wonder if we really need to set buf_dblwr to NULL after calling buf_dblwr_free() and fil_system to NULL after calling fil_close(). That seems to be redundant?

review: Needs Information

[Sergei Glushchenko (sergei.glushchenko)]

fil_init expect fil_system to be NULL, we will get assertion failure if we will try to init innodb once again.
As for buf_dblwr, I did it just for the case. If some part of code still trying to use it after I released it, I will get clear SEGFAULT crash instead of undefined behavior of trying to access memory which has been released.

[Alexey Kopytov (akopytov)]

My point was that buf_dblwr_free() sets buf_dblwr to NULL, and fil_close() sets fil_system to NULL, so there's no need to set them to NULL explicitly after calling the corresponding functions, i.e. that code is redundant.

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

I see now. Thanks for pointing it out.

http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-param/269/

[Alexey Kopytov (akopytov)]

Sergei,

Any idea what happens with galera-specific tests with ASAN enabled?

review: Needs Information

[Sergei Glushchenko (sergei.glushchenko)]

I am not an expert in Galera and SST, but PXC fails with following message:

2014-12-17 07:22:47 17194 [Warning] Buffered warning: Changed limits: table_cache: 431 (requested 2000)

2014-12-17 07:22:47 17194 [Note] WSREP: Read nil XID from storage engines, skipping position init
2014-12-17 07:22:47 17194 [Note] WSREP: wsrep_load(): loading provider library '/mnt/workspace/percona-xtrabackup-2.2-param/BUILD_TYPE/release/Host/asan/xtrabackuptarget/galera56/storage/innobase/xtrabackup/test/server/lib/libgalera_smm.so'
2014-12-17 07:22:47 17194 [ERROR] WSREP: wsrep_load(): dlopen(): libssl.so.10: cannot open shared object file: No such file or directory
2014-12-17 07:22:47 17194 [ERROR] WSREP: wsrep_load(/mnt/workspace/percona-xtrabackup-2.2-param/BUILD_TYPE/release/Host/asan/xtrabackuptarget/galera56/storage/innobase/xtrabackup/test/server/lib/libgalera_smm.so) failed: Invalid argument (22). Reverting to no provider.
2014-12-17 07:22:47 17194 [Note] WSREP: Read nil XID from storage engines, skipping position init
2014-12-17 07:22:47 17194 [Note] WSREP: wsrep_load(): loading provider library 'none'
2014-12-17 07:22:47 17194 [ERROR] Aborting

2014-12-17 07:22:47 17194 [Note] WSREP: Service disconnected.
2014-12-17 07:22:48 17194 [Note] WSREP: Some threads may fail to exit.
2014-12-17 07:22:48 17194 [Note] Binlog end
2014-12-17 07:22:48 17194 [Note] /mnt/workspace/percona-xtrabackup-2.2-param/BUILD_TYPE/release/Host/asan/xtrabackuptarget/galera56/storage/innobase/xtrabackup/test/server/bin//mysqld: Shutdown complete

2014-12-17 07:22:49: run.sh: ----------------

Looks like it did not try to run SST script.

[Alexey Kopytov (akopytov)]

Here's the reason:

2014-12-17 07:22:47 17194 [ERROR] WSREP: wsrep_load(): dlopen(): libssl.so.10: cannot open shared object file: No such file or directory
2014-12-17 07:22:47 17194 [ERROR] WSREP: wsrep_load(/mnt/workspace/percona-xtrabackup-2.2-param/BUILD_TYPE/release/Host/asan/xtrabackuptarget/galera56/storage/innobase/xtrabackup/test/server/lib/libgalera_smm.so) failed: Invalid argument (22). Reverting to no provider.

Which looks like another SSL-related build/packaging issue. Which is weird, but most likely has nothing to do with this fix.

review: Approve

[Sergei Glushchenko (sergei.glushchenko)]

Indeed, ubuntu 14.10 doesn't have libssl.so.10. It doesn't have libssl.so.6 also, which is required by PS tarball. How was it worked around?

I found race condition in xbstream_open which is not triggered in this Jenkins build, but is triggered in 2.3 build.

In code below
 if (stream_ctxt->dest_file == NULL) {
  stream_ctxt->dest_file = ds_open(dest_ctxt, path, mystat);
  if (stream_ctxt->dest_file == NULL) {
   return NULL;
  }
 }

stream_ctxt->dest_file could be opened twice in case of --parallel option.

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

I fixed the race condition

http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-param/270/

There is a crash on compact.sh with following message (debug):

This could be because you hit a bug or data is corrupted.
This error can also be caused by malfunctioning hardware.
We will try our best to scrape up some info that will hopefully help
diagnose the problem, but since we have already crashed,
something is definitely wrong and this may fail.

Thread pointer: 0x0
Attempting backtrace. You can use the following information to find out
where mysqld died. If you see no messages after this, something went
terribly wrong...
stack_bottom = 0 thread_stack 0x10000
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup(my_print_stacktrace+0x32) [0x874ec5b]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup(handle_fatal_signal+0x282) [0x86107a2]
[0xb76e8500]
/lib/libc.so.6(abort+0x17a) [0xb734c14a]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x847102e]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x8475296]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x84760ec]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x847a5db]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup(row_merge_build_indexes(trx_t*, dict_table_t*, dict_table_t*, bool, dict_index_t**, unsigned long const*, unsigned long, TABLE*, dtuple_t const*, unsigned long const*, unsigned long, ib_sequence_t&)+0x38b) [0x847f015]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x8223610]
/home/jenkins/workspace/percona-xtrabackup-2.3-param/BUILD_TYPE/debug/Host/centos6-32/xtrabackuptarget/galera55/storage/innobase/xtrabackup/src/xtrabackup() [0x8223988]
/lib/libpthread.so.0(+0x6b39) [0xb76c5b39]
/lib/libc.so.6(clone+0x5e) [0xb7402c1e]

I cannot reproduce it locally, but it doesn't look related to the fixes.
My best guess is that this is somewhere in row_merge_read_clustered_index or in row_merge_insert_index_tuples, rec pointer is NULL. To me it needs further investigation, but doesn't look related to the dix.

[Alexey Kopytov (akopytov)]

Sergei,

Doesn't ds_archive have the same problem as ds_xbstream?

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

Agree, ds_archive is similar in this respect. I made same fix for it.

http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-param/271/

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'storage/innobase/xtrabackup/src/compact.cc'
--- storage/innobase/xtrabackup/src/compact.cc	2014-09-21 18:38:17 +0000
+++ storage/innobase/xtrabackup/src/compact.cc	2014-12-22 11:29:31 +0000
@@ -419,6 +419,8 @@
 
 	rc = my_close(pmap_cur->fd, MY_WME);
 	xb_a(rc == 0);
+
+	my_free(pmap_cur);
 }
 
 /****************************************************************************

=== modified file 'storage/innobase/xtrabackup/src/ds_archive.c'
--- storage/innobase/xtrabackup/src/ds_archive.c	2013-11-26 10:44:44 +0000
+++ storage/innobase/xtrabackup/src/ds_archive.c	2014-12-22 11:29:31 +0000
@@ -27,6 +27,7 @@
 typedef struct {
 	struct archive	*archive;
 	ds_file_t	*dest_file;
+	pthread_mutex_t	mutex;
 } ds_archive_ctxt_t;
 
 typedef struct {
@@ -99,6 +100,10 @@
 			 MYF(MY_FAE));
 	archive_ctxt = (ds_archive_ctxt_t *)(ctxt + 1);
 
+	if (pthread_mutex_init(&archive_ctxt->mutex, NULL)) {
+		msg("archive_init: pthread_mutex_init() failed.\n");
+		goto err;
+	}
 
 	a = archive_write_new();
 	if (a == NULL) {
@@ -153,12 +158,14 @@
 
 	archive_ctxt = (ds_archive_ctxt_t *) ctxt->ptr;
 
+	pthread_mutex_lock(&archive_ctxt->mutex);
 	if (archive_ctxt->dest_file == NULL) {
 		archive_ctxt->dest_file = ds_open(dest_ctxt, path, mystat);
 		if (archive_ctxt->dest_file == NULL) {
 			return NULL;
 		}
 	}
+	pthread_mutex_unlock(&archive_ctxt->mutex);
 
 	file = (ds_file_t *) my_malloc(sizeof(ds_file_t) +
 				       sizeof(ds_archive_file_t),
@@ -262,5 +269,7 @@
 		archive_ctxt->dest_file = NULL;
 	}
 
+	pthread_mutex_destroy(&archive_ctxt->mutex);
+
 	my_free(ctxt);
 }

=== modified file 'storage/innobase/xtrabackup/src/ds_xbstream.c'
--- storage/innobase/xtrabackup/src/ds_xbstream.c	2013-11-26 10:44:44 +0000
+++ storage/innobase/xtrabackup/src/ds_xbstream.c	2014-12-22 11:29:31 +0000
@@ -27,6 +27,7 @@
 typedef struct {
 	xb_wstream_t	*xbstream;
 	ds_file_t	*dest_file;
+	pthread_mutex_t	mutex;
 } ds_stream_ctxt_t;
 
 typedef struct {
@@ -82,6 +83,11 @@
 			 MYF(MY_FAE));
 	stream_ctxt = (ds_stream_ctxt_t *)(ctxt + 1);
 
+	if (pthread_mutex_init(&stream_ctxt->mutex, NULL)) {
+		msg("xbstream_init: pthread_mutex_init() failed.\n");
+		goto err;
+	}
+
 	xbstream = xb_stream_write_new();
 	if (xbstream == NULL) {
 		msg("xb_stream_write_new() failed.\n");
@@ -116,12 +122,14 @@
 
 	stream_ctxt = (ds_stream_ctxt_t *) ctxt->ptr;
 
+	pthread_mutex_lock(&stream_ctxt->mutex);
 	if (stream_ctxt->dest_file == NULL) {
 		stream_ctxt->dest_file = ds_open(dest_ctxt, path, mystat);
 		if (stream_ctxt->dest_file == NULL) {
 			return NULL;
 		}
 	}
+	pthread_mutex_unlock(&stream_ctxt->mutex);
 
 	file = (ds_file_t *) my_malloc(sizeof(ds_file_t) +
 				       sizeof(ds_stream_file_t),
@@ -208,5 +216,8 @@
 		ds_close(stream_ctxt->dest_file);
 		stream_ctxt->dest_file = NULL;
 	}
+
+	pthread_mutex_destroy(&stream_ctxt->mutex);
+
 	my_free(ctxt);
 }

=== modified file 'storage/innobase/xtrabackup/src/xbcrypt_read.c'
--- storage/innobase/xtrabackup/src/xbcrypt_read.c	2014-02-28 14:37:59 +0000
+++ storage/innobase/xtrabackup/src/xbcrypt_read.c	2014-12-22 11:29:31 +0000
@@ -249,6 +249,8 @@
 {
 	if (crypt->buffer)
 		my_free(crypt->buffer);
+	if (crypt->ivbuffer)
+		my_free(crypt->ivbuffer);
 	my_free(crypt);
 
 	return 0;

=== modified file 'storage/innobase/xtrabackup/src/xtrabackup.cc'
--- storage/innobase/xtrabackup/src/xtrabackup.cc	2014-10-17 13:02:28 +0000
+++ storage/innobase/xtrabackup/src/xtrabackup.cc	2014-12-22 11:29:31 +0000
@@ -65,6 +65,7 @@
 #include <row0mysql.h>
 #include <row0quiesce.h>
 #include <srv0start.h>
+#include <buf0dblwr.h>
 
 #include <sstream>
 
@@ -2707,7 +2708,11 @@
 	os_aio_free();
 
 	fil_close_all_files();
-	fil_system = NULL;
+
+	/* Free the double write data structures. */
+	if (buf_dblwr) {
+		buf_dblwr_free();
+	}
 
 	/* Reset srv_file_io_threads to its default value to avoid confusing
 	warning on --prepare in innobase_start_or_create_for_mysql()*/
@@ -3675,6 +3680,7 @@
 	msg("\n");
 
 	os_event_free(log_copying_stop);
+	ds_close(dst_log_file);
 
 	/* Signal innobackupex that log copying has stopped and it may now
 	unlock tables, so we can possibly stream xtrabackup_logfile later
@@ -5578,6 +5584,15 @@
 	return xtrabackup_arch_first_file_lsn != 0;
 }
 
+static
+void
+innodb_free_param()
+{
+	srv_free_paths_and_sizes();
+	free(internal_innobase_data_file_path);
+	free_tmpdir(&mysql_tmpdir_list);
+}
+
 static void
 xtrabackup_prepare_func(void)
 {
@@ -5712,11 +5727,16 @@
 	}
 	sync_close();
 	sync_initialized = FALSE;
+	if (fil_system) {
+		fil_close();
+	}
 	os_sync_free();
 	mem_close();
 	os_sync_mutex = NULL;
 	ut_free_all_mem();
 
+	innodb_free_param();
+
 	/* Reset the configuration as it might have been changed by
 	xb_data_files_init(). */
 	if(innodb_init_param()) {