Bug #18435 5.0.19 libmysqlclient not ABI-compatible with 5.0.18
Submitted: 22 Mar 2006 20:03 Modified: 27 Mar 2006 8:59
Reporter: Lenz Grimmer Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S1 (Critical)
Version:5.0.19 OS:
Assigned to: Michael Widenius CPU Architecture:Any

[22 Mar 2006 20:03] Lenz Grimmer
Description:
This bug was reported on the packagers mailing list by Tom Lane. See the thread here:
http://lists.mysql.com/packagers/279

"5.0.19 has added a field to the MYSQL struct that was not there before.
For clients that use a MYSQL struct they have allocated themselves
(rather than allowing mysql_init to allocate it), this breaks binary
compatibility, because they won't have made the struct large enough."

$ diff -c mysql-5.0.18/include/mysql.h mysql-5.0.19/include/mysql.h
...
***************
*** 287,292 ****
--- 287,294 ----
      from mysql_stmt_close if close had to cancel result set of this object.
    */
    my_bool *unbuffered_fetch_owner;
+   /* needed for embedded server - no net buffer to store the 'info' */
+   char *info_buffer;
  } MYSQL;

  typedef struct st_mysql_res {
***************

I don't know why the MYSQL struct wasn't made abstract in the first
place, but given that mysql has chosen to expose it to client apps,
they can't change it (not this much anyway) without creating an ABI
break.

How to repeat:
Try to use a client built against MySQL 5.0.18 with the 5.0.19 shared libs (e.g. mysql-python as indicated in the discussion)

Suggested fix:
Either revert the incompatibility (so only 5.0.19 is the oddball), or increase the version of the client library when an incompatible change like this was done.
[22 Mar 2006 21:19] Michael Widenius
As info_buffer is not used in the not embedded version of libmysqlclient it should be safe to do a patch so that the extra info_buffer member is not used in clients < 5.1.0.

This will ensure that clients linked with either 5.0.18 or 5.0.19 should work with 5.0.20
[23 Mar 2006 5:08] [ name withheld ]
Hi Michael,

Unfortunately, it's not that easy because the 5.0.19 libmysqlclient *does* access the added field.  If it didn't, I'd never have noticed the problem ... but it tries to zero the field at mysql_init and then free() the pointer value at shutdown.  The crash I saw in mysql-python came about because mysql-python has its own pointer variable just after the MYSQL struct and the extra free() is inappropriate.
[23 Mar 2006 8:17] Michael Widenius
Thank you for your bug report. This issue has been committed to our
source repository of that product and will be incorporated into the
next release.

If necessary, you can access the source repository and build the latest
available version, including the bugfix, yourself. More information 
about accessing the source trees is available at
    http://www.mysql.com/doc/en/Installing_source_tree.html

Additional info:

Fix will be in 5.0.20
[25 Mar 2006 0:07] [ name withheld ]
I looked at the fix and don't find it makes me comfortable.  The problem is that the mysql.h file still exports a struct definition that is bigger than it was before, and this results in ABI breaks for anything incorporating that.  An example of the risk is that recompiling just one of the source files for mysql-python would result in a non-working mysql-python build, because that source file would have a different idea about sizeof(MYSQL) and thus about the offsets of fields in mysql-python's structs.

I really want to see something like #ifdef EMBEDDED_SERVER in the .h file, not only in the .c files.
[27 Mar 2006 8:59] Bugs System
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/4182
[27 Mar 2006 8:59] Michael Widenius
Thank you for your bug report. This issue has been committed to our
source repository of that product and will be incorporated into the
next release.

If necessary, you can access the source repository and build the latest
available version, including the bugfix, yourself. More information 
about accessing the source trees is available at
    http://www.mysql.com/doc/en/Installing_source_tree.html

Additional info:

In general there should not have beeen be any risk of
incompatibilities between releases after this patch.

The reason for this are:

- The new element in the MySQL structure is last and there is thus
  no changes in any offset for any old fields.

- It's normally perfectly safe to make a structure bigger as long as
  you don't write anything to the new members.  (The idea of the patch I
  made was to ensure we don't write anything to the new info_buffer
  member).

The fact that sizeof() changes does not affect any old program using
the shared library as they don't know anything about the size change.

Any new compiled program will pick up the new size, but this safe to use
both with a old and new libmysqlclient.so library.

The only incompatibility I can think of just now are:

a) If you have a client that have a lot of different parts that are
   compiled with different mysql.h header files (before and after the
   change) and you in one of them that is compiled with the newer size do
   a memset(&mysql, 0, sizeof(*mysql)) on a staticly allocated structure
   from a object file that uses the smaller version.
b) If you use the MYSQL structure as part of another global structure
   and the clients accessing this structure is compiled at different
   times with different header files.

Neither of the above is something that we recommend as this would
cause the clients to break for every major MySQL release and many
alpha and beta releases, while we are fine tuning structures.

The right way would be to use mysql_init() to allocate and populate
the MySQL structure. This would make the client safe against most
upgrades as we always try to do structure changes by adding things last
into the structures.

I did check of how MySQL python allocates it's structure and did
notice that it declares it connection structure the following way,
which is almost a conflict according to b):

typedef struct {
	PyObject_HEAD
	MYSQL connection;
	int open;
	PyObject *converter;
} _mysql_ConnectionObject;

I think the old patch is still safe for MySQL-python because
_mysql_ConnectionObject it's not a global structure and is thus not
used between different programs. In other words, there is no risk of
structure elements shifting as the offset are decided at compile time
and in this case only one mysql.h header is in use. The external size
of the MYSQL structure is also irrelevant as the MySQL client library
will not overwrite anything outside of the the old size of the MYSQL
structure.

I agree that in general to fix issue b), there is no other easy option
than to add an #ifdef EMBEDDED_SERVER around the new element in the
MySQL INFO structure :(

While doing the patch, I did not anticipate usage of the MySQL structure as
used by MySQL-python. To feel safer against things like this, I have
now added

#if defined(EMBEDDED_LIBRARY) || defined(EMBEDDED_LIBRARY_COMPATIBLE) || MYSQL_VERSION_ID >= 50100

around the new element in MYSQL. The define
"EMBEDDED_LIBRARY_COMPATIBLE" can be used by develeopers if they want
to e able to decide on link time to use the normal client library or
the embedded client library.

The other actions that should be taken are:

- We will contact they Python maintainer and ask them to change the
  MYSQL connection argument to be a pointer instead of a structure as
  this will make the connection object more safe against struct updates
  in the future.
- We will look at accelerate our own plans to make the MySQL external
  structures 'abstract' structures so that we can do changes in the
  structures without causing any incompatibilities with existing
  clients.

Thanks Tom for making us aware of the 'unexpected' way MySQL-Python allocates
and uses the MYSQL structure!

Regards,
Monty