• unknown's avatar
    A fix for Bug#29431 killing an insert delayed thread causes crash · e3b3f4ee
    unknown authored
    No test case, since the bug requires a stress case with 30 INSERT DELAYED
    threads and 1 killer thread to repeat. The patch is verified
    manually.
    Review fixes.
    
    The server that is running DELAYED inserts would deadlock itself
    or crash under high load if some of the delayed threads were KILLed
    in the meanwhile.
    
    The fix is to change internal lock acquisition order of delayed inserts
    subsystem and to ensure that
    Delayed_insert::table_list::db does not point to volatile memory in some 
    cases.
    For details, please see a comment for sql_insert.cc.
    
    
    sql/sql_insert.cc:
      A fix for Bug#29431 killing an insert delayed thread causes crash
      
      1) The deadlock was caused by different lock acquisition order
      between delayed_get_table and handle_delayed_insert.
      
      delayed_get_table would:
      - acquire LOCK_delayed_create
      - create a new Delayed_insert instance
      - acquire instance mutex (di->mutex)
      - "lock the instance in memory" by 
      increasing di->locked_in_memory variable
      - start the delayed thread
      - release di->mutex
      - let the delayed thread open the delayed table
      - discover that the delayed thread was killed
      - try to unlock() the delayed insert instance in memory
      - in Delayed_insert::unlock() do
       * lock LOCK_delayed_insert
       * decrease locks_in_memory and discover it's 0
       * attempt to lock di->mutex to broadcast di->cond condition <-- deadlock
       here
      
      Meanwhile, the delayed thread would
       * lock di->mutex
       * open the table
       * get killed
       * not notice that and attempt to lock LOCK_delayed_insert
      to register itself in the delayed insert list <-- deadlock here.
      
      Simply put, delayed_get_table used to lock LOCK_delayed_insert and then 
      di->mutex, and handle_delayed_insert would lock di->mutex and then 
      LOCK_delayed_insert.
      
      Fixed by moving registration in the list of delayed insert threads from 
      handle_delayed_insert to delayed_get_table - so that now
      handle_delayed_insert doesn't need to acquire LOCK_delayed_insert mutex.
      
      
      2) di->table_list.db was copied by-pointer from table_list.db of the first
      producer -- the one who initiated creation of the delayed insert thread.
      This producer might be long gone when the member is needed
      (handle_delayed_insert:open_ltable),
      e.g. by having been killed with KILL CONNECTION statement.
      Fixed by using a pointer to the consumer's deep copy of THD::db.
      
      3) In find_handler, we shouldn't assume that Delayed_insert object
      already (or still) has a table opened all the time it is
      present in the delayed insert list. E.g. it's not the case
      when Delayed_insert decided to terminate, closed the table, but haven't
      yet unregistered from the list (see the end of handle_delayed_insert).
    e3b3f4ee
sql_insert.cc 106 KB