Bug #57397 io_handler_thread() will never cleanup

• Submitted: 12 Oct 2010 8:37
• Modified: 30 Nov 2010 23:11
• Reporter: Stewart Smith
• Status: Closed
• Category: MySQL Server: InnoDB Plugin storage engine
• Severity: S3 (Non-critical)
• Version: 1.0.5+, 1.0.12
• OS: Any
• Assigned to: Jimmy Yang
• CPU Architecture: Any

[12 Oct 2010 8:37] Stewart Smith

Description:
In InnoDB Plugin 1.0.5 dev cycle, revision svn-v4:16c675df-0fcb-4bc9-8058-dcc011a37293:branches/zip:6130 added cleanup on shutdown of innodb plugin.

message:
  branches/zip: Free all resources at shutdown. Set pointers to NULL, so
  that Valgrind will not complain about freed data structures that are
  reachable via pointers.  This addresses Bug #45992 and Bug #46656.
  
  This patch is mostly based on changes copied from branches/embedded-1.0,
  mainly c5432, c3439, c3134, c2994, c2978, but also some other code was
  copied.  Some added cleanup code is specific to MySQL/InnoDB.
  
  rb://199 approved by Sunny Bains

SunStudio on Solaris rightly points out (see http://hudson.drizzle.org/view/Drizzle-param/job/drizzle-param/531/BUILD_TYPE=debug,label=... for a build)that 

        thr_local_free(os_thread_get_curr_id());

in srv0start.c:io_handler_thread() will never be reached.

The reason being that the above loop will never exit:

        for (i = 0;; i++) {
                fil_aio_wait(segment);

                mutex_enter(&ios_mutex);
                ios++;
                mutex_exit(&ios_mutex);
        }

How to repeat:
build with SunStudio

Suggested fix:
shutdown the thread, have condition checked, something like that.

[12 Oct 2010 13:45] Sveta Smirnova

Thank you for the report.

Verified as described.

To repeat I modified code as follow:

$bzr diff
=== modified file 'storage/innodb_plugin/srv/srv0start.c'
--- storage/innodb_plugin/srv/srv0start.c       2010-07-28 10:20:44 +0000
+++ storage/innodb_plugin/srv/srv0start.c       2010-10-12 13:41:03 +0000
@@ -471,6 +471,7 @@
        fprintf(stderr, "Io handler thread %lu starts, id %lu\n", segment,
                os_thread_pf(os_thread_get_curr_id()));
 #endif
+fprintf(stderr, "Here - start\n");
        for (i = 0;; i++) {
                fil_aio_wait(segment);
 
@@ -479,7 +480,9 @@
                mutex_exit(&ios_mutex);
        }
 
+fprintf(stderr, "Here\n");
        thr_local_free(os_thread_get_curr_id());
+fprintf(stderr, "Here - end\n");
 
        /* We count the number of threads in os_thread_exit(). A created
        thread should always use that to exit and not use return() to exit.

Then found only "Here - start"  had been printed into error log file.

[13 Oct 2010 8:39] Jimmy Yang

Most I/O-handler threads are exited in following stack:

#0  os_thread_exit
#1  os_event_wait_low 
#2  os_aio_simulated_handle
#3  fil_aio_wait 
#4  io_handler_thread

And original coding in io_handler_thread() also shows it will never exit the for() loop.

We modified the code as following to make the code look like exitable:

-	for (i = 0;; i++) {
+	while (srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS) {

Still most I/O hanlder thread will exit as in above stack. However, thr_local_free() is really not necessary, as thr_local_create() is not called for these threads.

[14 Oct 2010 4:21] Stewart Smith

makes sense, I've fixed ours in the same way too.

[13 Nov 2010 16:20] Bugs System

Pushed into mysql-trunk 5.6.99-m5 (revid:[e-mail]-20101113155825-czmva9kg4n31anmu) (version source revid:[e-mail]-20101113152450-2zzcm50e7i4j35v7) (merge vers: 5.6.1-m4) (pib:21)

[13 Nov 2010 16:40] Bugs System

Pushed into mysql-next-mr (revid:[e-mail]-20101113160336-atmtmfb3mzm4pz4i) (version source revid:[e-mail]-20100629074804-359l9m9gniauxr94) (pib:21)

[18 Nov 2010 15:55] Bugs System

Pushed into mysql-5.1 5.1.54 (revid:[e-mail]-20101118153531-693taxtxyxpt037i) (version source revid:[e-mail]-20101118153531-693taxtxyxpt037i) (merge vers: 5.1.54) (pib:21)

[16 Dec 2010 22:33] Bugs System

Pushed into mysql-5.5 5.5.9 (revid:[e-mail]-20101216101358-fyzr1epq95a3yett) (version source revid:[e-mail]-20101216101358-fyzr1epq95a3yett) (merge vers: 5.5.9) (pib:24)