Skip Menu |
 

This queue is for tickets about the POE CPAN distribution.

Report information
The Basics
Id: 29234
Status: rejected
Priority: 0/
Queue: POE

People
Owner: Nobody in particular
Requestors: perl [...] pied.nu
Cc:
AdminCc:

Bug Information
Severity: Unimportant
Broken in: 0.9999
Fixed in: (no value)



Subject: Specify aliases in POE::Session->create
Download (untitled) / with headers
text/plain 525b
It would be very nice to able to specify session aliases in POE::Session->create. As a string or arrayref. This would allow 2 things : - TRACE_SESSIONS (which happens before _start) could output something more useful then the session ID and stringified object. This, in turn, would be a big help answering the ubiquitous "Why won't the kernel exit?" question. - I suspect that most sessions have to call alias_set() at some point. I suspect that many sessions have a _start for the sole purpose of calling alias_set().
Download (untitled) / with headers
text/plain 215b
Preliminary patch to add alias functionality to session creation. I'm not sure if the patch is the right way to go, but if it is acceptable, ping me and I can I'll add tests/docs as well. Patched against SVN 2276.
Download rt-29234.patch
text/x-diff 3.5k
Index: trunk/poe/lib/POE/Kernel.pm =================================================================== --- trunk/poe/lib/POE/Kernel.pm (revision 2276) +++ trunk/poe/lib/POE/Kernel.pm (working copy) @@ -1368,7 +1368,7 @@ # structures as a side effect. sub session_alloc { - my ($self, $session, @args) = @_; + my ($self, $session, $aliases, @args) = @_; # If we already returned, then we must reinitialize. This is so # $poe_kernel->run() will work correctly more than once. @@ -1391,6 +1391,12 @@ # Register that a session was created. $kr_run_warning |= KR_RUN_SESSION; + # Set initial session alias if present + if (defined $aliases) { + $self->_alias_set($session, $_) + for @$aliases; + } + # Allocate the session's data structure. This must be done before # we dispatch anything regarding the new session. my $new_sid = $self->_data_sid_allocate(); @@ -2254,8 +2260,8 @@ ### Set an alias in the current session. -sub alias_set { - my ($self, $name) = @_; +sub _alias_set { + my ($self, $target_session, $name) = @_; if (ASSERT_USAGE) { _confess "<us> undefined alias in alias_set()" unless defined $name; @@ -2264,17 +2270,19 @@ # Don't overwrite another session's alias. my $existing_session = $self->_data_alias_resolve($name); if (defined $existing_session) { - if ($existing_session != $kr_active_session) { + if ($existing_session != $target_session) { $self->_explain_usage("alias '$name' is in use by another session"); return EEXIST; } return 0; } - $self->_data_alias_add($kr_active_session, $name); + $self->_data_alias_add($target_session, $name); return 0; } +sub alias_set { my $self = shift; $self->_alias_set($kr_active_session, @_) } + ### Remove an alias from the current session. sub alias_remove { Index: trunk/poe/lib/POE/Session.pm =================================================================== --- trunk/poe/lib/POE/Session.pm (revision 2276) +++ trunk/poe/lib/POE/Session.pm (working copy) @@ -20,6 +20,7 @@ sub CREATE_PACKAGES () { 'package_states' } sub CREATE_OBJECTS () { 'object_states' } sub CREATE_HEAP () { 'heap' } +sub CREATE_ALIAS () { 'aliases' } sub OPT_TRACE () { 'trace' } sub OPT_DEBUG () { 'debug' } @@ -184,13 +185,13 @@ } sub try_alloc { - my ($self, @args) = @_; + my ($self, $aliases, @args) = @_; # Verify that the session has a special start state, otherwise how # do we know what to do? Don't even bother registering the session # if the start state doesn't exist. if (exists $self->[SE_STATES]->{+EN_START}) { - $POE::Kernel::poe_kernel->session_alloc($self, @args); + $POE::Kernel::poe_kernel->session_alloc($self, $aliases, @args); } else { carp( "discarding session ", @@ -248,6 +249,22 @@ delete $params{+CREATE_OPTIONS}; } + # Extract initial aliases. + + my $aliases; + if (exists $params{+CREATE_ALIAS}) { + if (ref($params{+CREATE_ALIAS}) eq 'ARRAY') { + $aliases = $params{+CREATE_ALIAS}; + } + elsif (ref($params{+CREATE_ALIAS}) eq '') { + $aliases = [ $params{+CREATE_ALIAS} ]; + } + else { + croak "aliases for $type constructor is expected to be a ARRAY reference"; + } + delete $params{+CREATE_ALIAS}; + } + # Get down to the business of defining states. while (my ($param_name, $param_value) = each %params) { @@ -371,7 +388,7 @@ } } - return $self->try_alloc(@args); + return $self->try_alloc($aliases, @args); } #------------------------------------------------------------------------------
Download (untitled) / with headers
text/plain 501b
On Fri Mar 07 22:17:44 2008, PRAVUS wrote: Show quoted text
> Preliminary patch to add alias functionality to session creation. I'm > not sure if the patch is the right way to go, but if it is acceptable, > ping me and I can I'll add tests/docs as well. Patched against SVN 2276.
I would reject this patch. You've changed the call signature for alias_set() in a way that breaks all existing code. Also you've exposed a way to affect not-the-current-session in a public API. That's against POE::Kernel's religion.
Download (untitled) / with headers
text/plain 256b
I am rejecting this patch because it calls _data_alias_add() before the session is registered by _data_ses_allocate(). Modifying POE::Kernel data before the session has been registered is fragile design, and it opens a Pandora's box for future developers.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.