Title mod_mirrorbrain victim of incompatibility between apr-util 1.2 and 1.3
Priority urgent Status resolved
Superseder Nosy List dfarning, poeml
Assigned To poeml Keywords

Created on 2009-10-06.22:28:51 by poeml, last changed by poeml.

msg17 (view) Author: poeml Date: 2009-10-06.22:28:51
> the next issues....
> I am getting the following error report when trying to download

Ah. Interesting one.

> -----------------------
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] MirrorBrainEngine On, 
mirror_base '/var/www/downloads/', referer: http://mirrorbrain-
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] URI: 
'/sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] filename: 
'/var/www/downloads/sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
> [Tue Oct 06 11:53:46 2009] [error] [client] [mod_mirrorbrain] could not resolve continent, 
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] Country 'US', Continent '--', 
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] AS '--', Prefix '--', referer:
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] Successfully acquired database 
connection., referer:
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] SQL lookup for (canonicalized) 
'sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
> [Tue Oct 06 11:53:46 2009] [warn] [client] [mod_mirrorbrain] Found 2 mirrors, referer:

Apache successcully connects and talks to the database, and it does find the file on two
mirrors. That's great, and everything should work but...

> [Tue Oct 06 11:53:46 2009] [error] [client] (-1)Unknown error 18446744073709551615: 
[mod_mirrorbrain] Error looking up sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2 in database, referer:

...something else seems wrong, and I'm quite puzzled by this one.

In fact, after reading mod_mirrorbrain.c back and forth I'm quite sure
that this means that reading result lines from the database query fails.

The module correctly gets (and logs) the number of rows in the result,
but iterating over the result set fails (apr_dbd_get_row() call).

I suspect that the Apache Portable Runtime version of Ubuntu 9.04, and
the shipped PostgreSQL database adapter are a little bit too old (or
missing a bug fix) so it results in different behaviour.

Yes, this is probably the reason. On my Ubuntu 9.04 system I see a
libaprutil1 package with version 1.2.12, and in the ChangeLog I see:

- update to 1.3.0
  * ) All DBD drivers now count rows from 1, which affects PostgreSQL and MySQL
    drivers in particular. Using row number zero is an error.
    [Bojan Smojver]

That would explain the error.

I can probably come up with a workaround. Sorry about the inconvenience...
msg19 (view) Author: poeml Date: 2009-10-06.23:19:52
The following patch seemingly makes mod_mirrorbrain work with the old libaprutil:

--- mod_mirrorbrain.c   (revision 7798)
+++ mod_mirrorbrain.c   (working copy)
@@ -1127,7 +1127,7 @@
         const char *val = NULL;
         short col = 0; /* incremented for the column we are reading out */

-        rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, i);
+        rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, i-1);
         if (rv != 0) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
                       "[mod_mirrorbrain] Error looking up %s in database", filename);

I think we can quite easily change mod_mirrorbrain to work with both APR versions, 
either by detecting the APR version at build time, or by adding the patch to the Ubuntu

I don't know whether this fix works _well_, I can't really promise that at the 
moment, without looking into the database adapter (and its changelog) 
msg22 (view) Author: poeml Date: 2009-10-08.07:52:36
I'm adding the proposed fix to the Ubuntu/Debian packages.

I have not learnt yet how to properly add the change as patch when packaging Debian packages in the openSUSE build 
service, but a simple & stupid

    sed -i 's/&row, i);$$/\&row, i-1);/' mod_mirrorbrain.c

in the debian/rules file has the desired effect at the moment. Not nice, but for pragmatical reasons (also because 
I'll be away for vacation for a while) I'm fixing it this way now.
msg28 (view) Author: poeml Date: 2009-10-08.10:59:49
The packages with the fix are scheduled to be built in the openSUSE build service. They might 
turn up later today, but as the build service is extremely slow currently, I have put manually 
built packages here:
msg76 (view) Author: poeml Date: 2009-12-01.21:24:38
After sugarlabs is running with the fix for a month already, I think we can close 
this as fixed. 

And since it is easy to do compile time detection of the APR version, I'll commit 
a fix that does just that, so we can drop the patch from the Debian and Ubuntu 
msg77 (view) Author: poeml Date: 2009-12-01.21:32:27
Fixed in trunk:

Will be in the next release.
msg88 (view) Author: poeml Date: 2009-12-03.23:05:30
See also issue 29.
msg212 (view) Author: poeml Date: 2010-09-03.12:13:00
The check could be done at runtime, rather than at compile time.

That would protect the user in scenarios where a wrong package is installed (one 
that is compiled against a different APR version that what the system has).

There is apr_version() which returns the runtime APR version. See apr_version.h.

Apache uses it:
 # httpd2 -V | grep "APR "
Server loaded:  APR 1.4.2, APR-Util 1.3.9
Compiled using: APR 1.4.2, APR-Util 1.3.9

It would also allow us to log the versions to the error log at Apache's start.
msg215 (view) Author: poeml Date: 2010-09-05.19:01:23
mod_asn got a fix for this now (r86).
msg217 (view) Author: poeml Date: 2010-09-05.20:10:07
mod_mirrorbrain now has the same fix (r8107).
msg218 (view) Author: poeml Date: 2010-09-05.23:36:10
There is one remaining compile-time if-branch, which is not about database access 
though. It takes care of choosing %/d%lld as format string for (potentially) large 
integers in the SQL query (the one that retrieves hashes).  apr-1.2 knew only %d, 
while apr-1.3 knows %lld for 64-bit integers. 

#define DBD_LLD_FMT "d"
#define DBD_LLD_FMT "lld"

I'm leaving that as is, since the consequences are much less relevant (if it 
results in breakage at all). Hence, I close this issue as resolved.
Date User Action Args
2010-09-05 23:36:13poemlsetstatus: chatting -> resolved
messages: + msg218
2010-09-05 20:10:07poemlsetmessages: + msg217
2010-09-05 19:01:23poemlsetmessages: + msg215
2010-09-03 12:36:16poemlsettitle: mod_mirrorbrain incompatible with libaprutil 1.2 -> mod_mirrorbrain victim of incompatibility between apr-util 1.2 and 1.3
2010-09-03 12:13:01poemlsetstatus: resolved -> chatting
messages: + msg212
2009-12-03 23:05:36poemlsetstatus: chatting -> resolved
2009-12-03 23:05:30poemlsetstatus: resolved -> chatting
messages: + msg88
2009-12-01 21:34:13poemlsetstatus: chatting -> resolved
2009-12-01 21:32:28poemlsetstatus: resolved -> chatting
messages: + msg77
2009-12-01 21:24:40poemlsetstatus: testing -> resolved
nosy: + dfarning
messages: + msg76
2009-10-08 10:59:50poemlsetmessages: + msg28
2009-10-08 07:52:36poemlsetstatus: in-progress -> testing
messages: + msg22
2009-10-06 23:19:52poemlsetmessages: + msg19
2009-10-06 22:28:51poemlcreate