[users@httpd] RE: merging Apache context

2015-10-30 Thread Greenberg, Adam
Bill:

Thanks again for the quick response. The code I sent you is stripped down for 
demonstration purposes. We do normally handle the C++ allocation out of pools.

That said, I'd like to avoid any confusion C++ might introduce so I've written 
a C version that demonstrates the same behavior.

Here is the C code:

/*
 **
 *
 * @(#) modDirConfig2.c
 *
 * Copyright 2015, FMR Corp. All rights reserved.
 *
 **
 */

#include 

#include 
#include 
#include 

#include 
#include 
#include 

#include 
#include 

typedef struct
{
char * context;
char * message;
}  dir_config_rec;

static void * create_dc( apr_pool_t * pool, char * context )
{
if (NULL == context)
{
context = "unset";
}

// Create the configuration object.
dir_config_rec * cfg = apr_pcalloc( pool, sizeof( dir_config_rec ) );
if (NULL == cfg)
{
// An allocation error.
ap_log_perror( APLOG_MARK, APLOG_ERR, 0, pool,
 "Unable to allocate dir_config_rec for context: %s.",
 context );
return NULL;
}

cfg->context = apr_pstrdup( pool, context );
ap_log_perror( APLOG_MARK, APLOG_ERR, 0, pool,
"Created dir_config_rec for context: %s.", context );
return (void *) cfg;
}

static void * merge_dc( apr_pool_t * pool, void * parent, void * child )
{
dir_config_rec * p = (dir_config_rec *) parent;
dir_config_rec * c = (dir_config_rec *) child;
dir_config_rec * m = apr_pcalloc( pool, sizeof( dir_config_rec ) );

assert( NULL != p );
assert( NULL != c );
assert( NULL != m );

// Create a bread crumb context for the merged configuration.
m->context = apr_psprintf( pool, "%s:%s", p->context, c->context );

// Set the message to the child value.
m->message = c->message;

ap_log_perror( APLOG_MARK, APLOG_ERR, 0, pool,
"Merged config: %s", m->context );
return (void *) m;
}

static const char * insert( cmd_parms * parms, void * config,
const char * args )
{
if (NULL == parms)
{
return "insert NULL command parameters";
}

if (NULL == config)
{
return "insert NULL configuration";
}

if ((NULL == args) || ((* args) == '\0'))
{
return "insert NULL or empty arguments";
}

// Obtain the directory configuration.
dir_config_rec * dcfg = (dir_config_rec *) config;
dcfg->message = apr_pstrdup( parms->pool, args );
ap_log_error( APLOG_MARK, APLOG_ERR, 0, parms->server,
   "Inserted dir_config_rec with message: %s", args );
return NULL;
}

static const command_rec dirConfigDirectives[] =
{
AP_INIT_RAW_ARGS( "dirConfig", insert,
   NULL, ACCESS_CONF | RSRC_CONF,
  "Insert dirConfig" ),
{ NULL }
};

static apr_status_t hookFunc( request_rec * r );

static void registerHooks( apr_pool_t * pool )
{
ap_hook_fixups( hookFunc, NULL, NULL,
 APR_HOOK_MIDDLE );
}

/**
 * The Apache module declaration.
 */
AP_DECLARE_MODULE( DirConfig ) =
{
STANDARD20_MODULE_STUFF,
create_dc,
merge_dc,
NULL,
NULL,
dirConfigDirectives,
registerHooks
};

static apr_status_t hookFunc( request_rec * r )
{
dir_config_rec * dcfg =
(dir_config_rec *) ap_get_module_config( r->per_dir_config,
  & 
DirConfig_module );

ap_log_rerror( APLOG_MARK, APLOG_ERR, 0, r,
   "hookFunc: for request, %s, the context is: %s",
 r->uri, dcfg->context );
return DECLINED;
}

This emits the same console output:

bin/apachectl start
[Wed Dec 31 18:36:11.0,,0,. 1969] [DirConfig:error] [pid 21806] Created 
dir_config_rec for context: unset.
[Wed Dec 31 18:36:11.0-)))/ 1969] [DirConfig:error] [pid 21806] Created 
dir_config_rec for context: /foo/.
[Wed Dec 31 18:36:11.0-))+) 1969] [DirConfig:error] [pid 21806] Inserted 
dir_config_rec with message: /foo
[Wed Dec 31 18:36:11.0-)),( 1969] [DirConfig:error] [pid 21806] Created 
dir_config_rec for context: /.
[Wed Dec 31 18:36:11.0-)),. 1969] [DirConfig:error] [pid 21806] Inserted 
dir_config_rec with message: /
[Wed Dec 31 18:36:11.0-))-- 1969] [DirConfig:error] [pid 21806] Created 
dir_config_rec for context: /foo/bar/.
[Wed Dec 31 18:36:11.0-)).* 1969] [DirConfig:error] [pid 21806] Inserted 
dir_config_rec with message: /foo/bar
#

And the same two lines in the error log:

[Wed Dec 31 18:36:29.+),(,, 1969] [DirConfig:error] [pid 21809] Merged config: 
unset:/
...
[Wed Dec 31 18:36:29.+),).. 1969] [DirConfig:error] [pid 21809] [client 
10.27.15.3:54410] hookFunc: for request, /foo/bar/foo.html, the context is: 
unset:/

Apache appears to behave differently from what I woul

[users@httpd] Re: merging Apache context

2015-10-30 Thread William A. Rowe Jr.
On Fri, Oct 30, 2015 at 5:23 PM, Greenberg, Adam  wrote:
> Hi Bill (this is a resend to include the dev and user communities, per your
> instructions):

Sorry for any misunderstanding - this seems like a good users@ question
(with some C++ thrown in!) so I'm dropping the dev@ list. FWIW attaching
the source file as text instead of a tar file lets others quickly examine and
participate in the discussion, posts with archive attachments are often
ignored for lack of time, even if they might be interesting to other readers.

> Thanks very much for the prompt response. I believe we have covered all of
> the steps you indicate below.

Yea, someone should steal that troubleshooting list and throw it on the wiki.
Might be useful to Nick if he publishes an update to the module authoring
volume.

> Attached please find a tar file that contains
> a simple C++ module and a makefile to build it.
>
> Our test server http.conf has a LoadModule line for the module and then the
> following set of directory sections:
>
> 
>   dirConfig /foo
> 
>
> 
>   dirConfig /
> 
>
> 
>   dirConfig /foo/bar
> 
>
> When we execute the server, we see this on the console:
> # bin/apachectl start
> Created dirConfig for context: unset
> Created dirConfig for context: /foo/
> Inserted dirConfig with message: /foo
> Created dirConfig for context: /
> Inserted dirConfig with message: /
> Created dirConfig for context: /foo/bar/
> Inserted dirConfig with message: /foo/bar
> Created dirConfig for context: unset
>
> And this in the log:
> Merged config: unset:/
> hookFunc: for request /foo/bar/foo.html the context is: unset:/
>
> We do not see any other messages. The file /foo/bar/foo.html exists and the
> browser displays it. This suggests that the merge sequence for the last
> directory section stopped with the configuration for the “/” section.
> Perhaps you could point out what would cause this behavior?

As I mentioned when I had a chance to teach this material, server
configs are almost
exclusively allocated at startup time out of the pconf - configuration
pool.  It is a pool
whose contents should -never- change for the lifetime of the server
worker processes.
But it *does* change in the parent process, when a restart is
requested, the existing
pconf is dumped and a new pconf is allocated, and the configuration is re-read.

But dir configs are a different beast, they may be created at runtime
(for example,
.htaccess file contents), merged for the top request, all internal
redirects, and all
of the subrequests (mod_autoindex or mod_dav_fs can cause thousands of these
to present just one file listing).  They absolutely must be allocated
from the pool
passed to the create() or merge() handler.

In your C++ code, you have;

void * dirConfig::create_dc( apr_pool_t * pool, char * context )
{
  // Create the configuration object.
  dirConfig * cfg = new dirConfig();
  if (NULL == cfg)
  {
// An allocation error.
return NULL;
  }
  // Keep the context name.
  std::string temp = "unset";

  if (NULL != context)
  {
temp = context;
  }
  cfg->setContext( temp );

  std::cerr << "Created dirConfig for context: " << temp << std::endl;
  return (void *) cfg;
}

Here's the first problem... you allocated dirConfig() but didn't allocate this
from the given pool.  You then returned a naked pointer to the C++ object,
and I have no idea offhand whether that permanently increases the object's
use count, or whether the dirConfig object is then released immediately
upon the return from the create_dc function.

You can register a pool cleanup against this pool which destroys the
given dirConfig, but that would be a costly proposition performance-wise.
Do this only if it must be a managed type, and that should only be for
your per-request or per-connection objects.  You should handle these
httpd structures (server config, dir config, etc) as unmanaged "C" data
and let the pool schema perform your cleanups for you.

You probably want to start with this change to watch lifetimes;
dirConfig::~dirConfig()
{
  std::cerr << "Destroyed dirConfig of context: " << _context << std::endl;
}

You also may want to watch your objects a little more closely by emitting
the memory address of 'this' itself, so you can determine that the object
being modified is the object created, both in your directive handler and
in the merge function, etc.

Maybe another dev/user who has coded some C++ modules can offer
some more ideas and insight here.  I wrote mod_aspdotnet under MS
managed C++.NET years ago and ran into many issues where I had
to properly unbox and re-box the managed data when passing it back
and forth from unmanaged httpd "C" code.  That source code lives
http://sourceforge.net/p/mod-aspdotnet/code/HEAD/tree/mod_aspdotnet2/trunk/
if it is of some help to compare how I handled similar issues.

Enjoy your weekend...

Yours,

Bill



> From: William A. Rowe Jr. [mailto:wr...@pivotal.io]
> Sent: Thursday, October 29, 2015 6:04 PM
> To: PAN, JIN
> Cc: wr...@apache.org

[users@httpd] RE: merging Apache context

2015-10-30 Thread Greenberg, Adam
Hi Bill (this is a resend to include the dev and user communities, per your 
instructions):

Thanks very much for the prompt response. I believe we have covered all of the 
steps you indicate below. Attached please find a tar file that contains a 
simple C++ module and a makefile to build it.

Our test server http.conf has a LoadModule line for the module and then the 
following set of directory sections:


  dirConfig /foo



  dirConfig /



  dirConfig /foo/bar


When we execute the server, we see this on the console:

# bin/apachectl start
Created dirConfig for context: unset
Created dirConfig for context: /foo/
Inserted dirConfig with message: /foo
Created dirConfig for context: /
Inserted dirConfig with message: /
Created dirConfig for context: /foo/bar/
Inserted dirConfig with message: /foo/bar
Created dirConfig for context: unset

And this in the log:

Merged config: unset:/
…
hookFunc: for request /foo/bar/foo.html the context is: unset:/

We do not see any other messages. The file /foo/bar/foo.html exists and the 
browser displays it. This suggests that the merge sequence for the last 
directory section stopped with the configuration for the “/” section.  Perhaps 
you could point out what would cause this behavior?

Thanks:
Adam


From: William A. Rowe Jr. [mailto:wr...@pivotal.io]
Sent: Thursday, October 29, 2015 6:04 PM
To: PAN, JIN
Cc: wr...@apache.org; Greenberg, Adam
Subject: Re: merging Apache  context

Hi Jin,

there might be more than one thing going on here.

First, it is critical that a directive belonging to the module occurs in each 
of the  blocks you are merging.

Remember httpd is not going to even create a config section, never mind merge 
them, for every module whose directives do not appear in a given  - 
this is what makes httpd so efficient.

Second, if there is a bug in the cmd handler, your ctx member might not be 
correctly updated for a section, same is true for a bug in the create or merge 
function.

Third, httpd does perform some optimization, it may premerge global configs and 
may resume a merge from previously merged sections when they are encountered in 
a subrequest.

Is the resulting ->ctx member correct for the resulting  context?  
Is it simply that the merge isn't called as often as expected?  Optimization 
may be the cause.

Is the cmd record for your directive set to OR_ACCESS (telling httpd that it is 
a per-dir and not per-server config?)

Is there a bug in your create code that is returning NULL instead of a newly 
initialized config section?

If we look at the example of mod_dir.c, here are the key points...


AP_DECLARE_MODULE(dir) = {

STANDARD20_MODULE_STUFF,

create_dir_config,  /* create per-directory config structure */

merge_dir_configs,  /* merge per-directory config structures */
All is well, we have a create + merge handler...


static void *create_dir_config(apr_pool_t *p, char *dummy)

{

dir_config_rec *new = apr_pcalloc(p, sizeof(dir_config_rec));



new->index_names = NULL;

new->do_slash = MODDIR_UNSET;

new->checkhandler = MODDIR_UNSET;

new->redirect_index = REDIRECT_UNSET;

return (void *) new;

}
the correct structure size is created and members initialized to empty (e.g. 
'unset') - the new allocation is returned.


static void *merge_dir_configs(apr_pool_t *p, void *basev, void *addv)

{

dir_config_rec *new = apr_pcalloc(p, sizeof(dir_config_rec));

dir_config_rec *base = (dir_config_rec *)basev;

dir_config_rec *add = (dir_config_rec *)addv;



new->index_names = add->index_names ? add->index_names : base->index_names;

new->do_slash =

(add->do_slash == MODDIR_UNSET) ? base->do_slash : add->do_slash;

new->checkhandler =

(add->checkhandler == MODDIR_UNSET) ? base->checkhandler : 
add->checkhandler;

new->redirect_index=

(add->redirect_index == REDIRECT_UNSET) ? base->redirect_index : 
add->redirect_index;

new->dflt = add->dflt ? add->dflt : base->dflt;

return new;

}
A new config is created, the various per-dir values updated, and the resulting 
new allocation is returned.


static const command_rec dir_cmds[] =

{

...

AP_INIT_RAW_ARGS("DirectoryIndex", add_index, NULL, DIR_CMD_PERMS,

"a list of file names"),
The DIR_CMD_PERMS (defined as OR_INDEXES) assures httpd that this is a per-dir 
config directive allowed wherever the 'AllowOverride Indexes' is set.


static const char *add_index(cmd_parms *cmd, void *dummy, const char *arg)

{

dir_config_rec *d = dummy;

const char *t, *w;

int count = 0;



if (!d->index_names) {

d->index_names = apr_array_make(cmd->pool, 2, sizeof(char *));

}



t = arg;

while ((w = ap_getword_conf(cmd->pool, &t)) && w[0]) {

if (count == 0 && !strcasecmp(w, "disabled")) {

/* peek to see if "disabled" is first in a series of arguments */

const char *tt = t;

const char *ww = ap_