Re: #30201 [Com]: crash with query cache when killing a select

[Kristofer Pettersson]

Hi,

> --- a/sql/sql_cache.cc	2007-05-10 17:54:53 +05:00
> +++ b/sql/sql_cache.cc	2007-08-22 11:07:03 +05:00
> @@ -706,6 +706,12 @@ void query_cache_end_of_result(THD *thd)
>    Query_cache_block *query_block;
>    DBUG_ENTER("query_cache_end_of_result");
>  
> +  if (thd->killed)
> +  {
> +    query_cache_abort(&thd->net);
> +    DBUG_VOID_RETURN;
> +  }
> +

Ok, query_cache_abort checks null-pointers and removes query cache blocks from store.

>    /* See the comment on double-check locking usage above. */
>      DBUG_VOID_RETURN;
> @@ -727,13 +733,10 @@ void query_cache_end_of_result(THD *thd)
>      DUMP(&query_cache);
>      BLOCK_LOCK_WR(query_block);
> -    Query_cache_block *last_result_block= header->result()->prev;
> -    ulong allign_size= ALIGN_SIZE(last_result_block->used);
> -    ulong len= max(query_cache.min_allocation_unit, allign_size);
> -    if (last_result_block->length >= query_cache.min_allocation_unit + len)
> -      query_cache.split_block(last_result_block,len);
> +    Query_cache_block *last_result_block;
> +    ulong allign_size;
> +    ulong len;
>  
> -#ifndef DBUG_OFF
>      if (header->result() == 0)
>      {
>        DBUG_PRINT("error", ("end of data whith no result. query '%s'",
> @@ -747,7 +750,13 @@ void query_cache_end_of_result(THD *thd)
>        */
>        goto end;
>      }
> -#endif
> +
> +    last_result_block= header->result()->prev;
> +    allign_size= ALIGN_SIZE(last_result_block->used);
> +    len= max(query_cache.min_allocation_unit, allign_size);
> +    if (last_result_block->length >= query_cache.min_allocation_unit + len)
> +      query_cache.split_block(last_result_block,len);
> +
>      header->found_rows(current_thd->limit_found_rows);
>      header->result()->type= Query_cache_block::RESULT;
>      header->writer(0);

Ok: check if we have a result block before we reference it. But if there is a query cache
block
in the store which doesn't have any result set this is inconvenient later on in
send_result_to_client:
.
.
.
  query = query_block->query();
  result_block= first_result_block= query->result();

  if (result_block == 0 || result_block->type != Query_cache_block::RESULT)
  {
    /* The query is probably yet processed */
    DBUG_PRINT("qcache", ("query found, but no data or data incomplete"));
    BLOCK_UNLOCK_RD(query_block);
    goto err_unlock;
  }
.
.
.

Better to remove this query block at once when we detected the error. Please change from:
    if (header->result() == 0)
    {
      DBUG_PRINT("error", ("end of data whith no result. query '%s'",
                           header->query()));
      query_cache.wreck(__LINE__, "");  <-- disables the query cache which is a bit
unnecessary

      /*
        We do not need call of BLOCK_UNLOCK_WR(query_block); here because
        query_cache.wreck() switched query cache off but left content
        untouched for investigation (it is debugging method).
      */
      goto end;
    }

Change to:

    if (header->result() == 0)
    {
      DBUG_PRINT("error", ("End of data with no result blocks; Query '%s' removed from
cache.",
                           header->query()));
	
      BLOCK_UNLOCK_WR(query_block);
      query_cache.free_query(query);
      
      goto end;
    }

Cheers,

Kristofer