You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philip M. Gollucci" <pg...@p6m7g8.com> on 2006/01/09 20:21:19 UTC

Connection pnotes

Geoff,

http://aspn.activestate.com/ASPN/Mail/Message/modperl/2871864

I never saw this get committed .......... ?

What needs to happen for that ?

-- 
------------------------------------------------------------------------
"Love is not the one you can picture yourself marrying,
but the one you can't picture the rest of your life without."

"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."

"I wanna hold ya till I die ... I wanna hold ya till the fear in me 
subsides."

Philip M. Gollucci (pgollucci@p6m7g8.com) 301.254.5198
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F  6A47 D55A 7172 A799 97F

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
> ConnectionUtil.xs: In function 'XS_Apache2__Connection_pnotes':
> ConnectionUtil.xs:36: warning: implicit declaration of function 'mpxs_Apache2__Connection_pnotes'
> ConnectionUtil.xs:36: warning: assignment makes pointer from integer without a cast
> make[3]: *** [ConnectionUtil.o] Error 1
I vaguely remember getting this before.  See the archives.  I believe 
patch is failing to make a directory thus a file missing and the above 
error.

fyi, when I tested and I did on abotu 30 combos, it looked good
+1 from me.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Geoffrey Young wrote:
> --- /dev/null	2006-01-27 10:40:15.572121608 -0500
> +++ xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h	2005-10-25 13:33:27.000000000 -0400
AFAIK, the only thing that was a problem is that
Apache2/ConnectionUtil isn't an SVN dir.  So when you apply the patch 
locally, 'patch' the program, barfs and puts this file in the current 
directory (probably the top level mp2 source tree).  Simply making the 
directory before running patch should solve the issues.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Fred Moyer wrote:
> Philippe M. Chiasson wrote:
>> Geoffrey Young wrote:
>>>>> it will probably be easier for me to muck with the tests once they are
>>>>> committed.  to that end, let's get the functionality to where it needs to be
>>>>> then I'll work on recreating and removing the warnings.
>>>> If the patch looks good to you, I'll check it in and we should be good.
>>> yeah, go ahead.
>>
>> We now have connection pnotes as of r386784.
>>
>> Documentation patch is still necessary.
> 
> Here's a suggested patch.  I've created a separate pod file for 
> Apache2::ConnectionUtil.  I've referenced Apache2::RequestUtil::pnotes, 
> shown usage, and identified differences between the two.  Style and 
> wording matched to Apache2::RequestUtil pnotes method for the usage.

Thanks, added as revision 396942.
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5


Re: Connection pnotes

Posted by Fred Moyer <fr...@taperfriendlymusic.org>.
Philippe M. Chiasson wrote:
> Geoffrey Young wrote:
>>>>it will probably be easier for me to muck with the tests once they are
>>>>committed.  to that end, let's get the functionality to where it needs to be
>>>>then I'll work on recreating and removing the warnings.
>>>
>>>If the patch looks good to you, I'll check it in and we should be good.
>>
>>yeah, go ahead.
> 
> 
> We now have connection pnotes as of r386784.
> 
> Documentation patch is still necessary.

Here's a suggested patch.  I've created a separate pod file for 
Apache2::ConnectionUtil.  I've referenced Apache2::RequestUtil::pnotes, 
shown usage, and identified differences between the two.  Style and 
wording matched to Apache2::RequestUtil pnotes method for the usage.


Index: src/docs/2.0/api/config.cfg
===================================================================
--- src/docs/2.0/api/config.cfg (revision 396758)
+++ src/docs/2.0/api/config.cfg (working copy)
@@ -21,6 +21,7 @@
          Apache2/CmdParms.pod
          Apache2/Command.pod
          Apache2/Connection.pod
+        Apache2/ConnectionUtil.pod
          Apache2/Const.pod
          Apache2/Directive.pod
          Apache2/Filter.pod

src/docs/2.0/api/Apache2/ConnectionUtil.pod
=======================================================================
+++ src/docs/2.0/api/Apache2/ConnectionUtil.pod (new copy)
=head1 NAME

Apache2::ConnectionUtil - Perl API for Apache connection utils




=head1 Synopsis

   use Apache2::Connection     ();
   use Apache2::ConnectionUtil ();
   use Apache2::RequestRec     ();

   # grab the connection object;
   my $c = $r->connection;

   # share perl objects like $r->pnotes
   $old_val = $c->pnotes($key => $value);






=head1 Description

C<Apache2::ConnectionUtil> provides the
L<Apache connection record object|docs::2.0::api::Apache2::Connection>
utilities API.





=head1 API

C<Apache2::ConnectionUtil> provides the following functions and/or
methods:








=head2 C<pnotes>

Share Perl variables between requests over the lifetime of the
connection.

   $old_val  = $c->pnotes($key => $val);
   $val      = $c->pnotes($key);
   $hash_ref = $c->pnotes();



=over 4

=item obj: C<$c>
( C<L<Apache2::Connection object|docs::2.0::api::Apache2::Connection>> )

=item opt arg1: C<$key> ( string )

A key value

=item opt arg2: C<$val> ( SCALAR )

Any scalar value (e.g. a reference to an array)

=item ret: (3 different possible values)

if both, C<$key> and C<$val> are passed the previous value for C<$key> is
returned if such existed, otherwise undef is returned.

if only C<$key> is passed, the current value for the given key is returned.

if no arguments are passed, a hash reference is returned, which can then 
be directly accessed without going through the C<pnotes()> interface.

=item since: 2.0.3

=back

See 
(C<L<Apache2::RequestUtil|docs::2.0::api::Apache2::RequestUtil/C_pnotes_>>)
for the details of the C<pnotes> method usage.  The usage is identical
except for a few differences.  First is the use of C<$c> instead of 
C<$r> as
the invocant.  The second is that the the data persists for the lifetime of
the connection instead of the lifetime of the request.  If the connection is
lost, so is the data stored in C<pnotes>.



=head1 See Also

L<Apache2::Connection|docs::2.0::api::Apache2::Connection>.

L<Apache2::RequestUtil|docs::2.0::api::Apache2::RequestUtil/C_pnotes_>.

L<mod_perl 2.0 documentation|docs::2.0::index>.


=head1 Copyright

mod_perl 2.0 and its core modules are copyrighted under
The Apache Software License, Version 2.0.




=head1 Authors

L<The mod_perl development team and numerous
contributors|about::contributors::people>.

=cut


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Geoffrey Young wrote:
>>This seems to make the problem go away, but I am not sure if that's where it
>>belongs:
>>
>>sub handler {
>>
>>    my $r = shift;
>>
>>    # make it ok to call ok() here while plan()ing elsewhere
>>    Apache::Test::init_test_pm($r);
>>
>>+   Test::_reset_globals() if Test->can('_reset_globals');
>>    $Test::ntest   = 1 + (22 * ($r->args - 1));
>>    $Test::planned = 22;
> 
> gozer++
> 
>>
>>>it will probably be easier for me to muck with the tests once they are
>>>committed.  to that end, let's get the functionality to where it needs to be
>>>then I'll work on recreating and removing the warnings.
>>
>>If the patch looks good to you, I'll check it in and we should be good.
> 
> yeah, go ahead.

We now have connection pnotes as of r386784.

Documentation patch is still necessary.

>>Oh, and I forgot one other benefit of the patch, it merges in the pool cleanup
>>patch I send before, removing the pnotes cleanup from mod_perl's guts and into
>>a pool cleanup only when used.
> 
> excellent.  way to find the tuits, philippe.

I do what I can ;-)

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> This seems to make the problem go away, but I am not sure if that's where it
> belongs:
> 
> sub handler {
> 
>     my $r = shift;
> 
>     # make it ok to call ok() here while plan()ing elsewhere
>     Apache::Test::init_test_pm($r);
> 
> +   Test::_reset_globals() if Test->can('_reset_globals');
>     $Test::ntest   = 1 + (22 * ($r->args - 1));
>     $Test::planned = 22;

gozer++

> 
> 
>>it will probably be easier for me to muck with the tests once they are
>>committed.  to that end, let's get the functionality to where it needs to be
>>then I'll work on recreating and removing the warnings.
> 
> 
> If the patch looks good to you, I'll check it in and we should be good.

yeah, go ahead.

> 
> Oh, and I forgot one other benefit of the patch, it merges in the pool cleanup
> patch I send before, removing the pnotes cleanup from mod_perl's guts and into
> a pool cleanup only when used.

excellent.  way to find the tuits, philippe.

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Geoffrey Young wrote:
>>Passes all tests, but I see tons (66 to be precise) of this warning in the
>>error_log, geoff ?
>>
>>"Use of uninitialized value in caller at /usr/lib/perl5/5.8.6/Test.pm line 374."
> 
> yeah, I suspect that's because I'm playing around with how things are
> planned, etc.

This seems to make the problem go away, but I am not sure if that's where it
belongs:

sub handler {

    my $r = shift;

    # make it ok to call ok() here while plan()ing elsewhere
    Apache::Test::init_test_pm($r);

+   Test::_reset_globals() if Test->can('_reset_globals');
    $Test::ntest   = 1 + (22 * ($r->args - 1));
    $Test::planned = 22;

> it will probably be easier for me to muck with the tests once they are
> committed.  to that end, let's get the functionality to where it needs to be
> then I'll work on recreating and removing the warnings.

If the patch looks good to you, I'll check it in and we should be good.

Oh, and I forgot one other benefit of the patch, it merges in the pool cleanup
patch I send before, removing the pnotes cleanup from mod_perl's guts and into
a pool cleanup only when used.

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Passes all tests, but I see tons (66 to be precise) of this warning in the
> error_log, geoff ?
> 
> "Use of uninitialized value in caller at /usr/lib/perl5/5.8.6/Test.pm line 374."

yeah, I suspect that's because I'm playing around with how things are
planned, etc.

it will probably be easier for me to muck with the tests once they are
committed.  to that end, let's get the functionality to where it needs to be
then I'll work on recreating and removing the warnings.

fair?

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Philip M. Gollucci wrote:
>>I'll work something up when I have the chance... in a month or two :)
> 
> God do I know the feeling.
> 
> Thanks for the good eyes Gozer.

No problem, and attached is a modified patch that:

1 - Doesn't leak a HV * on each pass
2 - Merges the pnotes logic between r->pnotes & c->pnotes in one piece of code

Passes all tests, but I see tons (66 to be precise) of this warning in the
error_log, geoff ?

"Use of uninitialized value in caller at /usr/lib/perl5/5.8.6/Test.pm line 374."

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
> I'll work something up when I have the chance... in a month or two :)
God do I know the feeling.

Thanks for the good eyes Gozer.

-- 
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323 219 4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F  6A47 D55A 7172 A799 97F

"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> What I mean is that you are leaking a HV. Every connection, if someone uses
> c->pnotes, you will run this code:
> 
>     if (!ccfg->pnotes) {
>         ccfg->pnotes = newHV();
>     }
> 
> And you have nothing in place to cleanup this newly created HV (and it's contents)

ah, yeah.  d'oh!

I'll work something up when I have the chance... in a month or two :)

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Geoffrey Young wrote:
>>All tests pass for me on various platforms, but...
>>
>>Where do you cleanup HV *modperl_config_con_t->pnotes ? Looks to me
>>like you are leaking it. You'd need something similar to what we
>>are doing for request pnotes:
>>
>>apr_status_t modperl_config_request_cleanup(pTHX_ request_rec *r)
>>{
>>    [...]
>>
>>    if (rcfg->pnotes) {
>>        SvREFCNT_dec(rcfg->pnotes);
>>        rcfg->pnotes = Nullhv;
>>    }
> 
> I have some comments on that in the code
> 
>     /* note we do not need to implement a per-connection cleanup to
>      * reset $c->pnotes.  this is because we don't hook into
>      * ap_hook_create_connection and therefore each new connection
>      * has no &perl_module data in the configuration vector
> 
> however, refcounting isn't really my thing, so I'll accept your analysis if
> you think mine above is insufficient (which it may well be, since you're
> smarter about these things than I am :)

What I mean is that you are leaking a HV. Every connection, if someone uses
c->pnotes, you will run this code:

    if (!ccfg->pnotes) {
        ccfg->pnotes = newHV();
    }

And you have nothing in place to cleanup this newly created HV (and it's contents)

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> All tests pass for me on various platforms, but...
> 
> Where do you cleanup HV *modperl_config_con_t->pnotes ? Looks to me
> like you are leaking it. You'd need something similar to what we
> are doing for request pnotes:
> 
> apr_status_t modperl_config_request_cleanup(pTHX_ request_rec *r)
> {
>     [...]
> 
>     if (rcfg->pnotes) {
>         SvREFCNT_dec(rcfg->pnotes);
>         rcfg->pnotes = Nullhv;
>     }

I have some comments on that in the code

    /* note we do not need to implement a per-connection cleanup to
     * reset $c->pnotes.  this is because we don't hook into
     * ap_hook_create_connection and therefore each new connection
     * has no &perl_module data in the configuration vector

however, refcounting isn't really my thing, so I'll accept your analysis if
you think mine above is insufficient (which it may well be, since you're
smarter about these things than I am :)

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Geoffrey Young wrote:
>>-    plan $r, tests => 9;
>>+    # make it ok to call ok() here while plan()ing elsewhere
>>+    Apache::Test::init_test_pm($r);
>>+    $Test::ntest   = 1 + (22 * ($r->args - 1));
>>+    $Test::planned = 22;
> 
> and of course everyone is wondering what this is.  it would help if my
> (third) patch actually included all the bits and pieces I put together for this.
> 
> part of the issue is that t/modperl/pnotes.t is in the svn ignore list, so
> svn status didn't pick it up...
> 
> anyway, sorry for the flood of spam, but _this_ should be good to go.
> really, I should have caught both mistakes when philip said things looked
> good but the first few iterations of the patch didn't include the real
> functionality :)  guess I'm having a significant amount of brane
> deterioration from not working on this stuff as much as I ought...
> 
> so, testing on a battery of platforms appreciated.

All tests pass for me on various platforms, but...

Where do you cleanup HV *modperl_config_con_t->pnotes ? Looks to me
like you are leaking it. You'd need something similar to what we
are doing for request pnotes:

apr_status_t modperl_config_request_cleanup(pTHX_ request_rec *r)
{
    [...]

    if (rcfg->pnotes) {
        SvREFCNT_dec(rcfg->pnotes);
        rcfg->pnotes = Nullhv;
    }

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> -    plan $r, tests => 9;
> +    # make it ok to call ok() here while plan()ing elsewhere
> +    Apache::Test::init_test_pm($r);
> +    $Test::ntest   = 1 + (22 * ($r->args - 1));
> +    $Test::planned = 22;

and of course everyone is wondering what this is.  it would help if my
(third) patch actually included all the bits and pieces I put together for this.

part of the issue is that t/modperl/pnotes.t is in the svn ignore list, so
svn status didn't pick it up...

anyway, sorry for the flood of spam, but _this_ should be good to go.
really, I should have caught both mistakes when philip said things looked
good but the first few iterations of the patch didn't include the real
functionality :)  guess I'm having a significant amount of brane
deterioration from not working on this stuff as much as I ought...

so, testing on a battery of platforms appreciated.

--Geoff

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>I think I was waiting for him to get back to me with some real test results,
>>which IIRC he said he would do when he could upgrade :)
>>
>>remarks on the list as to the thread safety made me a bit nervous about
>>commiting this particular bit, just because I don't have a clue when it
>>comes to threads.  but if someone with a clue takes a look and think it
>>looks ok I'll be happy to commit.
> 
> 
> looks like something is missing from that patch:
> 
> ConnectionUtil.xs: In function 'XS_Apache2__Connection_pnotes':
> ConnectionUtil.xs:36: warning: implicit declaration of function 'mpxs_Apache2__Connection_pnotes'
> ConnectionUtil.xs:36: warning: assignment makes pointer from integer without a cast
> make[3]: *** [ConnectionUtil.o] Error 1

blarg, did I forget the meat of the patch last time?

here's a new one to test out.

--Geoff

Re: Connection pnotes

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Geoffrey Young wrote:
> 
> Philip M. Gollucci wrote:
> 
>>Geoff,
>>
>>http://aspn.activestate.com/ASPN/Mail/Message/modperl/2871864
>>
>>I never saw this get committed .......... ?
>>
>>What needs to happen for that ?
> 
> 
> I think I was waiting for him to get back to me with some real test results,
> which IIRC he said he would do when he could upgrade :)
> 
> remarks on the list as to the thread safety made me a bit nervous about
> commiting this particular bit, just because I don't have a clue when it
> comes to threads.  but if someone with a clue takes a look and think it
> looks ok I'll be happy to commit.

looks like something is missing from that patch:

ConnectionUtil.xs: In function 'XS_Apache2__Connection_pnotes':
ConnectionUtil.xs:36: warning: implicit declaration of function 'mpxs_Apache2__Connection_pnotes'
ConnectionUtil.xs:36: warning: assignment makes pointer from integer without a cast
make[3]: *** [ConnectionUtil.o] Error 1

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: Connection pnotes

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

Philip M. Gollucci wrote:
> Geoff,
> 
> http://aspn.activestate.com/ASPN/Mail/Message/modperl/2871864
> 
> I never saw this get committed .......... ?
> 
> What needs to happen for that ?

I think I was waiting for him to get back to me with some real test results,
which IIRC he said he would do when he could upgrade :)

remarks on the list as to the thread safety made me a bit nervous about
commiting this particular bit, just because I don't have a clue when it
comes to threads.  but if someone with a clue takes a look and think it
looks ok I'll be happy to commit.

--Geoff

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org