MySQL Bugs: #33247: mysqlbinlog does not clean up after itself on abnormal termination

Bug #33247	mysqlbinlog does not clean up after itself on abnormal termination
Submitted:	14 Dec 2007 14:40	Modified:	16 Feb 2008 11:45
Reporter:	Sven Sandberg	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Replication	Severity:	S3 (Non-critical)
Version:	5.1	OS:	Any
Assigned to:	Sven Sandberg	CPU Architecture:	Any
Tags:	Abort, destroy, exit, FREE, memory leak, mysqlbinlog

Description:
When mysqlbinlog terminates abnormally, e.g., because an invalid log event is found, it does not free memory after itself.

How to repeat:
Run the test case binlog_base64_flag

Suggested fix:
Remove the die() function from mysqlbinlog. Instead, print an error message and return failure. Propagate returned failures up to main(). Make sure everything that is ever allocated, is later free'd.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/41508

ChangeSet@1.2679, 2008-01-31 16:18:51+01:00, sven@riska.(none) +5 -0
  BUG#33247: mysqlbinlog does not clean up after itself on abnormal termination
  Problem: mysqlbinlog does not free memory if an error happens.
  Fix: binlog-processing functions do not call exit() anymore. Instead, they
  print an error and return an error code. Error codes are propagated all
  the way back to main, and all allocated memory is freed on the way.

== New error handling ======

To make mysqlbinlog clean up properly after itself, we need to change
the way it handles errors.  Currently, the program may die in the
middle without freeing memory.  In order to free memory allocated in a
caller of the failing function, we should not die on error, but
instead print an error message and return an error status from the
function.  Error status should be propagated from callee to caller and
only in main should the program exit.

In general, there are three ways that a binlog-processing function
in mysqlbinlog can exit.  In the fix, I have created an enumeration
type called Exit_status which indicates how the caller should
proceed:

 - Execution should continue normally. This is the normal return
   value.

 - Execution should stop and main should return 1. This is returned
   if there is an error.

 - Execution should stop and main should return 0. This happens
   when the events to be printed are bounded by
   --stop-position=X or --stop-datetime=X.

In the fix, I also made all error messages be printed as soon as the
error happens.

== New errors ======

Several errors that were not taken care of, are now handled. E.g.:

 - I/O error when processing Append_block

 - I/O error when writing base64 event

 - Error returned from process_event() in check_header

 - Out of memory constructing Format_description_log_event

Every time the program terminates with exit code 1, there is now an
error message.

All error messages now end with '.'.  Some spelling errors have been
fixed.

== Minor code clean-up ======

The error message "Attempting to dump binlog which was not closed
properly" was in main because it needs to know about the filename of
the currently active binlog.  In order to move the printing of this
error message to where the error is detected, I added the filename of
the currently active binlog as a parameter to some functions.

mysqlbinlog maintains the two global variables MYSQL* mysql and
Format_description_log_event* glob_description_event.  These are
sometimes referred to directly and sometimes passed as parameter.
Since the parameter always takes the value of the global variable, I
have removed the parameter.  Now we always refer to the global
variable.

I have added comments to all functions that I have modified.

I renamed check_database to shall_skip_database, since the latter
better describes what the function does.

== How memory management worksa ======

With error handling as above, it is easy to ensure that all memory is
freed.

By default, the responsibility to free memory lies in the
topmost-level function that has a reference to it.  Exceptions are:

 - Log_events.  These are either freed by the function that creates
   them, or passed to process_event().  process_event() frees the
   event, unless it needs to be saved for later. There are two event
   types that need to be saved for later:

    - Format_description_log_events are saved in the global variable
      glob_description_event and freed when the next
      Format_description_log_event is seen.  When the program
      terminates, the current glob_description_event is freed.

    - Create_file_query_log_events are saved in the global object
      load_processor and freed when the corresponding
      Execute_load_log_event is seen.  When the program terminates, it
      looks for any remaining Create_file_query_log_events still in
      the load_processor and frees them.

 - Some global variables are freed on program termination, in the
   function cleanup().

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/41721

ChangeSet@1.2679, 2008-02-05 16:55:44+01:00, sven@riska.(none) +5 -0
  BUG#33247: mysqlbinlog does not clean up after itself on abnormal termination
  Problem: mysqlbinlog does not free memory if an error happens.
  Fix: binlog-processing functions do not call exit() anymore. Instead, they
  print an error and return an error code. Error codes are propagated all
  the way back to main, and all allocated memory is freed on the way.

The second patch is like the first patch, but with the following modifications:

 - load_processor.process(Append_block_log_event) returns OK_CONTINUE if the file_id for the event was not registered by a previous Begin or Create_file event.

 - All doxygen comments are now complete with description, parameters, and return values.

 - A hard-coded 1 was replaced by the symbolic name MY_REPLACE_DIR in the call to fn_format from Load_log_processor::prepare_new_file_for_old_format

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/41957

ChangeSet@1.2544, 2008-02-08 18:17:00+01:00, sven@riska.(none) +5 -0
  BUG#33247: mysqlbinlog does not clean up after itself on abnormal termination
  Problem: mysqlbinlog does not free memory if an error happens.
  Fix: binlog-processing functions do not call exit() anymore. Instead, they
  print an error and return an error code. Error codes are propagated all
  the way back to main, and all allocated memory is freed on the way.

Pushed into 5.1.24-rc

Pushed into 6.0.5-alpha

Documented bugfix in the 5.1.24 and 6.0.5 changelogs as follows:

        mysqlbinlog failed to release all of its memory after
        terminating abnormally.

Also documented for 5.1.23-ndb-6.2.14.

Also documented for 5.1.23-ndb-6.3.11.