Re: conn_rec needs a context

[Jim Jagielski]

Yes, that is right. SOME modules can grab the context for *other*
modules (proxy does this), but in general, the context is related
to *this* connection for *this* module.

> On Mar 3, 2016, at 10:30 AM, Stefan Eissing  
> wrote:
> 
> 
>> Am 03.03.2016 um 16:21 schrieb Yann Ylavic :
>> 
>> On Thu, Mar 3, 2016 at 3:57 PM, Jim Jagielski  wrote:
>>>   /** Config vector containing pointers to connections per-server
>>>*  config structures. */
>>>   struct ap_conf_vector_t *conn_config;
>> 
>> Yes, this is opaque, and turns out to be a implemented as void**.
>> We could even make it a struct if we'd want 2 "batons" per module,
>> with something like:
>> struct ap_conf_vector_t {
>> void *config;
>> void *context;
>> };
>> in server/config.c, without breaking the API (since it is opaque).
>> And then let ap_get_module_config/context() use the existing
>> conn/request_config.
>> Another vector might be better/simpler/safer, though.
>> 
>> Still I'm not sure to understand the use case for now...
> 
> I think we confuse ourselves with the wording here. As I understand it, 
> almost all modules store connection context information in that vector. In 
> that sense, it is *their* internal configuration for the connection, not 
> necessarily the same as is stored at other config locations, such as server 
> or request.
> 
> Some ap_conf_vectors are populated by the runtime for all modules 
> participating (like server_rec->module_config), some are not. The ones in 
> conn_rec ->conn_config are only directly set by a module and the module 
> determines what does there.
> And what a module stores there can be a process wide singleton or something 
> with the lifetime of the conn_rec->pool, it's up to the module.
> 
> That is how I understand it. And I hope that I'm right, but if not, please 
> explain what I missed, since mod_http2 makes very much use of this...
> 
> -Stefan
> 

Re: conn_rec needs a context

[Yann Ylavic]

On Thu, Mar 3, 2016 at 4:30 PM, Stefan Eissing
 wrote:
>
> Some ap_conf_vectors are populated by the runtime for all modules 
> participating (like server_rec->module_config), some are not. The ones in 
> conn_rec ->conn_config are only directly set by a module and the module 
> determines what does there.
> And what a module stores there can be a process wide singleton or something 
> with the lifetime of the conn_rec->pool, it's up to the module.

+1

Re: conn_rec needs a context

[Stefan Eissing]

> Am 03.03.2016 um 16:21 schrieb Yann Ylavic :
> 
> On Thu, Mar 3, 2016 at 3:57 PM, Jim Jagielski  wrote:
>>/** Config vector containing pointers to connections per-server
>> *  config structures. */
>>struct ap_conf_vector_t *conn_config;
> 
> Yes, this is opaque, and turns out to be a implemented as void**.
> We could even make it a struct if we'd want 2 "batons" per module,
> with something like:
>  struct ap_conf_vector_t {
>  void *config;
>  void *context;
>  };
> in server/config.c, without breaking the API (since it is opaque).
> And then let ap_get_module_config/context() use the existing
> conn/request_config.
> Another vector might be better/simpler/safer, though.
> 
> Still I'm not sure to understand the use case for now...

I think we confuse ourselves with the wording here. As I understand it, almost 
all modules store connection context information in that vector. In that sense, 
it is *their* internal configuration for the connection, not necessarily the 
same as is stored at other config locations, such as server or request.

Some ap_conf_vectors are populated by the runtime for all modules participating 
(like server_rec->module_config), some are not. The ones in conn_rec 
->conn_config are only directly set by a module and the module determines what 
does there.
And what a module stores there can be a process wide singleton or something 
with the lifetime of the conn_rec->pool, it's up to the module.

That is how I understand it. And I hope that I'm right, but if not, please 
explain what I missed, since mod_http2 makes very much use of this...

-Stefan

Re: conn_rec needs a context

[Jim Jagielski]

Forgot to mention that it's created w/ ptrans which is c->pool

> On Mar 3, 2016, at 9:57 AM, Jim Jagielski  wrote:
> 
>/** Config vector containing pointers to connections per-server
> *  config structures. */
>struct ap_conf_vector_t *conn_config;
> 

Re: conn_rec needs a context

[Yann Ylavic]

On Thu, Mar 3, 2016 at 3:57 PM, Jim Jagielski  wrote:
> /** Config vector containing pointers to connections per-server
>  *  config structures. */
> struct ap_conf_vector_t *conn_config;

Yes, this is opaque, and turns out to be a implemented as void**.
We could even make it a struct if we'd want 2 "batons" per module,
with something like:
  struct ap_conf_vector_t {
  void *config;
  void *context;
  };
in server/config.c, without breaking the API (since it is opaque).
And then let ap_get_module_config/context() use the existing
conn/request_config.
Another vector might be better/simpler/safer, though.

Still I'm not sure to understand the use case for now...

Re: conn_rec needs a context

[Jim Jagielski]

/** Config vector containing pointers to connections per-server
 *  config structures. */
struct ap_conf_vector_t *conn_config;

Re: conn_rec needs a context

[Yann Ylavic]

On Wed, Mar 2, 2016 at 5:20 PM, Graham Leggett  wrote:
> On 02 Mar 2016, at 2:26 AM, Yann Ylavic  wrote:
>
 Would it make sense to add a vector of contexts that same way we have a 
 vector of configs, one slot for each module, which will allow any module 
 to add a context of it’s own to conn_rec without having to extend conn_rec?
>>>
>>> Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
>>> used for that purpose?
>>
>> I mean, put the context in the module config?
>
> If we can it would be great - but is there an API guarantee that the memory 
> pointed to by the config is a per-connection copy of the config, and not the 
> raw underlying server wide config instead?

Hmm, not sure to follow...
The server_config is indeed pconf wide, per_dir_config is either pconf
(untouched) or request (merges) wide, but conn_config and
request_config are to be allocated explicitly by each module on the
relevant pool, nothing automagic for them AFAICT.

Would you like a per conn/request context which is automatically
allocated for all the modules, and which does not collide with the
existing conn/request configs eventually used by the modules?
Where would ap_get_module_context(c->conn_context, ) be
called from, in some "core" code or by each module?
In the former case I'd see the need for it (I suspect this is what you want).
But otherwise it could be handled by each module in its own _config.
Hard to say w/o more details or code...

FWIW, the http_module has no conn/request_config so far, couldn't you
simply use:
   ctx = ap_get_module_config(c->conn_config, _module)
in the ap_process_http_connection() hook?

Regards,
Yann.

Re: conn_rec needs a context

[Stefan Eissing]

> Am 02.03.2016 um 17:20 schrieb Graham Leggett :
> 
> On 02 Mar 2016, at 2:26 AM, Yann Ylavic  wrote:
> 
 Would it make sense to add a vector of contexts that same way we have a 
 vector of configs, one slot for each module, which will allow any module 
 to add a context of it’s own to conn_rec without having to extend conn_rec?
>>> 
>>> Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
>>> used for that purpose?
>> 
>> I mean, put the context in the module config?
> 
> If we can it would be great - but is there an API guarantee that the memory 
> pointed to by the config is a per-connection copy of the config, and not the 
> raw underlying server wide config instead?

Does not, for example, mod_ssl rely on that?

Re: conn_rec needs a context

[Graham Leggett]

On 02 Mar 2016, at 2:26 AM, Yann Ylavic  wrote:

>>> Would it make sense to add a vector of contexts that same way we have a 
>>> vector of configs, one slot for each module, which will allow any module to 
>>> add a context of it’s own to conn_rec without having to extend conn_rec?
>> 
>> Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
>> used for that purpose?
> 
> I mean, put the context in the module config?

If we can it would be great - but is there an API guarantee that the memory 
pointed to by the config is a per-connection copy of the config, and not the 
raw underlying server wide config instead?

Regards,
Graham
—

Re: conn_rec needs a context

[Stefan Eissing]

+1

> Am 02.03.2016 um 12:21 schrieb Jim Jagielski :
> 
> 
>> On Mar 1, 2016, at 7:09 PM, Graham Leggett  wrote:
>> 
>> Hi all,
>> 
>> In order to get connections to have async behaviour, it must be possible for 
>> the process_connection hook to exit in the expectation of being called again 
>> when an async mpm is present - this is easy, the mpms already do that.
>> 
>> The missing bit is that conn_rec needs a context so that when is is called a 
>> second time it knows what state it is in.
>> 
>> There is a void *ctx variable that was added in r1565657, but I’m not sure 
>> it is enough.
>> 
>> Would it make sense to add a vector of contexts that same way we have a 
>> vector of configs, one slot for each module, which will allow any module to 
>> add a context of it’s own to conn_rec without having to extend conn_rec?
>> 
>> It would be nice to have the same for requests, for the same reason.
>> 
> 
> Makes sense to me...
> 

Re: conn_rec needs a context

[Jim Jagielski]

> On Mar 1, 2016, at 7:09 PM, Graham Leggett  wrote:
> 
> Hi all,
> 
> In order to get connections to have async behaviour, it must be possible for 
> the process_connection hook to exit in the expectation of being called again 
> when an async mpm is present - this is easy, the mpms already do that.
> 
> The missing bit is that conn_rec needs a context so that when is is called a 
> second time it knows what state it is in.
> 
> There is a void *ctx variable that was added in r1565657, but I’m not sure it 
> is enough.
> 
> Would it make sense to add a vector of contexts that same way we have a 
> vector of configs, one slot for each module, which will allow any module to 
> add a context of it’s own to conn_rec without having to extend conn_rec?
> 
> It would be nice to have the same for requests, for the same reason.
> 

Makes sense to me...

Re: conn_rec needs a context

[Yann Ylavic]

On Wed, Mar 2, 2016 at 1:23 AM, Yann Ylavic  wrote:
> Hi,
>
> On Wed, Mar 2, 2016 at 1:09 AM, Graham Leggett  wrote:
>>
>> Would it make sense to add a vector of contexts that same way we have a 
>> vector of configs, one slot for each module, which will allow any module to 
>> add a context of it’s own to conn_rec without having to extend conn_rec?
>
> Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
> used for that purpose?

I mean, put the context in the module config?

Re: conn_rec needs a context

[Yann Ylavic]

Hi,

On Wed, Mar 2, 2016 at 1:09 AM, Graham Leggett  wrote:
>
> Would it make sense to add a vector of contexts that same way we have a 
> vector of configs, one slot for each module, which will allow any module to 
> add a context of it’s own to conn_rec without having to extend conn_rec?

Can't the existing ap_[gs]et_module_config(c->conn_config, ...) be
used for that purpose?

>
> It would be nice to have the same for requests, for the same reason.

Likewise with r->request_config?

Regards,
Yann.