[
https://issues.jboss.org/browse/MODCLUSTER-349?page=com.atlassian.jira.pl...
]
Michal Babacek commented on MODCLUSTER-349:
-------------------------------------------
Hi guys, I'm sorry, this patch looks wrong to me. It appears to be doing what it
should, but IMHO it doesn't allocate enough space for the incoming message, because
{{MAXMESSSIZE}} is not enough.
I've used a fake client (not the one attached to this JIRA) that sends these messages:
*CONFIG*, *ENABLE-APP* with 500 aliases and finally *STATUS*.
Note that the
[
mod_cluster.conf|https://gist.github.com/Karm/343bfd9b2ccc921bd458#file-m...]
had been left unchanged during both tests.
This first test corresponds to the following scenario:
# start httpd 2.2.22 with mod_cluster 1.0.10.GA_CP04 from EAP 5.2 distribution
# GET mod_cluster-manager console, check for HTTP 200
# send *CONFIG* messages
# read response
# send *ENABLE-APP* messages
# read response
# send *STATUS* messages
# read responses
# retrieve and archive mod_cluster-manager console content
# stop httpd
# backup mod_manager.so
# replace mod_manager.so with the patched one from [JBPAPP-10851]
# repeat steps 1 - 10 (with a new mod_manager.so)
# retrieve mod_manager.so from backup and repeat steps 1 - 10
You might observe in the [first scenario
record|https://gist.github.com/Karm/343bfd9b2ccc921bd458/raw/504567e14448...]
that with the first run, only 361 aliases were correctly loaded, with the 362nd one being
corrupted and the rest missing whatsoever. If you follow that record further, you might
see that the patched version managed to display 376 aliases correctly. That's better,
yet, not really satisfying. Furthermore, at the end of the record, the original version
did again 361 aliases. This third iteration serves merely as a sanity check. Here is the
corresponding
[
error_log|https://gist.github.com/Karm/343bfd9b2ccc921bd458/raw/43e165ea8...].
The second test corresponds to the aforementioned scenario but for the mod_cluster
versions. Instead of 1.0.10.GA_CP04, 1.2.4 was used during the first and third iteration.
For the second iteration, mod_manager.so was replaced with the one from mod_cluster 1.2.6.
The results are dramatically different:
You might observe in the [second scenario
record|https://gist.github.com/Karm/343bfd9b2ccc921bd458/raw/243f5a00a4ab...]
that with the first iteration, again only 361 aliases were correctly loaded, whereas for
the second iteration, the one with mod_manager from 1.2.6, it was the correct 500 aliases
value. At the end of the record, the third iteration brought the same number of aliases as
the first one: 361.
Here is the corresponding
[
error_log|https://gist.github.com/Karm/343bfd9b2ccc921bd458/raw/97dbbd5ca...].
(i) It is noteworthy, that in the middle of that log, *ENABLE-APP* message ends with
{{alias-372.e}} by the time mod_cluster-console reported correctly 500 loaded aliases. It
looks like {{buff}} does not contain what it should or is not displayed correctly:
{code}
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"manager_handler %s (%s) processing: \"%s\"",
r->method, r->filename, buff);
{code}
Anyhow, I think that that this patch:
{code}
- status = ap_get_brigade(r->input_filters, input_brigade, AP_MODE_READBYTES,
APR_BLOCK_READ, MAXMESSSIZE);
+
+ while ((status = ap_get_brigade(r->input_filters, input_brigade,
AP_MODE_READBYTES, APR_BLOCK_READ, len)) == APR_SUCCESS) {
+ apr_brigade_flatten(input_brigade, buff + bufsiz, &len);
+ apr_brigade_cleanup(input_brigade);
+ bufsiz += len;
+ if (bufsiz >= MAXMESSSIZE || len == 0) break;
+ len = MAXMESSSIZE - bufsiz;
+ }
+
{code}
is not correct and that calculating the buffer size from all the relevant elements as in
mod_cluster 1.2.6 is better:
{code}
/* Use a buffer to read the message */
if (mconf->maxmesssize)
maxbufsiz = mconf->maxmesssize;
else {
/* we calculate it */
maxbufsiz = 9 + JVMROUTESZ;
maxbufsiz = bufsiz + (mconf->maxhost * HOSTALIASZ) + 7;
maxbufsiz = bufsiz + (mconf->maxcontext * CONTEXTSZ) + 8;
}
if (maxbufsiz< MAXMESSSIZE)
maxbufsiz = MAXMESSSIZE;
buff = apr_pcalloc(r->pool, maxbufsiz);
input_brigade = apr_brigade_create(r->pool, r->connection->bucket_alloc);
len = maxbufsiz;
while ((status = ap_get_brigade(r->input_filters, input_brigade, AP_MODE_READBYTES,
APR_BLOCK_READ, len)) == APR_SUCCESS) {
apr_brigade_flatten(input_brigade, buff + bufsiz, &len);
apr_brigade_cleanup(input_brigade);
bufsiz += len;
if (bufsiz >= maxbufsiz || len == 0) break;
len = maxbufsiz - bufsiz;
}
{code}
What do you think about the results?
mod_manager truncates ENABLE-APP messages
-----------------------------------------
Key: MODCLUSTER-349
URL:
https://issues.jboss.org/browse/MODCLUSTER-349
Project: mod_cluster
Issue Type: Bug
Affects Versions: MOD_CLUSTER_1_0_10_GA_CP04, 1.2.4.Final
Environment: Windows Server 2008
JBoss EAP 5.1.0
EWS 1.0.1
Reporter: Gregory Lardiere
Assignee: Jean-Frederic Clere
Fix For: 1.2.5.Final
Attachments: ModCluster349.java, mod_cluster.patch
With lots of aliases, ENABLE-APP messages can be big enough to span several IP packets.
Sometimes, mod_manager will read only the first packet.
The message is therefore incomplete and mod_cluster configuration ends up corrupted :
some aliases are missing and the context-root is replaced by "/".
This is caused by a wrong usage of ap_get_brigade() : it's called only once whereas
it should be called until there's nothing left to read (see the attached patch).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see:
http://www.atlassian.com/software/jira