search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 551 forks source link

Don't rebless a IO::Socket unless you need to [rt.cpan.org #53606] #17443

Open toddr opened 6 years ago

toddr commented 6 years ago

Migrated from rt.cpan.org#53606 (status was 'open')

Requestors:

From leonerd-cpan@leonerd.org.uk on 2010-01-11 20:14:20:

I have the following class heirarchy:

  IO::Socket
   +-- IO::Socket::Netlink
        +-- IO::Socket::Netlink::Route

When I try to

  my $rtnlsock = IO::Socket::Netlink::Route->new()

the line in IO::Socket::configure() kicks in:

  bless($sock, $domain2pkg[$domain]);

which down-blesses my object out of the ::Route subclass, so it loses
specific methods relating to Route-based sockets.

Could you instead change this to something like:

  my $pkg = $domain2pkg[$domain];
  bless($sock, $pkg) unless $sock->isa($pkg);

That way, when I've called the constructor on a _subclass_ of what is
already the right class of IO::Socket, it doesn't break.

-- 

Paul Evans

From leonerd-cpan@leonerd.org.uk on 2010-01-11 20:15:46:

On Mon Jan 11 15:14:20 2010, PEVANS wrote:
>   IO::Socket
>    +-- IO::Socket::Netlink
>         +-- IO::Socket::Netlink::Route

(Apologies; rt.cpan seems to strip whitespace here making my diagram
less-than-clear)

-- 

Paul Evans

From gbarr@pobox.com on 2010-01-11 20:37:20:

On Jan 11, 2010, at 2:14 PM, Paul Evans via RT wrote:
> 
> I have the following class heirarchy:
> 
>  IO::Socket
>   +-- IO::Socket::Netlink
>        +-- IO::Socket::Netlink::Route
> 
> When I try to
> 
>  my $rtnlsock = IO::Socket::Netlink::Route->new()
> 
> the line in IO::Socket::configure() kicks in:
> 
>  bless($sock, $domain2pkg[$domain]);
> 
> which down-blesses my object out of the ::Route subclass, so it loses
> specific methods relating to Route-based sockets.
> 
> Could you instead change this to something like:
> 
>  my $pkg = $domain2pkg[$domain];
>  bless($sock, $pkg) unless $sock->isa($pkg);

Any subclass of IO::Socket should define its own configure, otherwise you will end up
with a loop due to the line that follows the bless

    bless($sock, $domain2pkg[$domain]);
    $sock->configure($arg);

IO::Socket::configure was added to allow

  IO::Socket->new(Domain => $domain);

to work by re-blessing into the package for the socket domain

IO::Socket::Netlink needs to define a configure method, even if it does nothing

Graham.

From leonerd-cpan@leonerd.org.uk on 2010-01-15 19:10:42:

On Mon Jan 11 15:37:20 2010, gbarr@pobox.com wrote:
> Any subclass of IO::Socket should define its own configure, otherwise
> you will end up
> with a loop due to the line that follows the bless
> 
>     bless($sock, $domain2pkg[$domain]);
>     $sock->configure($arg);
...
> IO::Socket::Netlink needs to define a configure method, even if it
> does nothing

It does contain one of these. That doesn't help. See below.

> IO::Socket::configure was added to allow
> 
>   IO::Socket->new(Domain => $domain);
> 
> to work by re-blessing into the package for the socket domain

That's fine. That's an up-cast; a rebless into a -more- specific class.

The problematic blessing that's going on here is a down-cast; a rebless
into a -less- specific class, one which loses information.

Given an IO::Socket::Netlink which has been mistakenly reblessed by
IO::Socket::configure, it can work out which actual subclass it should
be, by inspecting the Protocol field (analogous to what IO::Socket
itself is doing with the domain). 

But that doesn't help ultimately. Most Netlink sockets will in fact be
Netlink::Generic, or rather subclasses of them.

There is no fact about a Netlink::Generic object which it can inspect to
see which way to up-cast itself. E.g. a ::Taskstats socket just happens
to have certain messages passed into the kernel, and expect certain
replies or broadcasts.

It really would be best not to break the object's own identity like this.

Can I perhaps instead suggest replacing:

     bless($sock, $domain2pkg[$domain]);
     $sock->configure($arg);

With something like:

    my $pkg = $domain2pkg[$domain];
    if( $sock->isa( $pkg ) ) {
        $pkg->can( "configure" )->( $sock, $arg );
    }
    else {
        bless($sock, $pkg);
        $sock->configure($arg);
    }

This will not alter the up-cast case for normal use in e.g.
IO::Socket->new( Domain => AF_INET );  but it will then mean that when
specific subclass constructors have already been called, the sockets
they construct won't lose their identity in a way that cannot be
reconstructed.

-- 

Paul Evans

From leonerd-cpan@leonerd.org.uk on 2010-01-15 19:34:51:

Actually, scrub all that. I've just realised the configure() method
doesn't get called on subclasses anyway.

Instead I was relying on the fact that my configure() would get called for

  my $sock = IO::Socket::Netlink::Taskstats->new;

when in fact it doesn't, because there are no arguments:

 48     return scalar(%arg) ? $sock->configure(\%arg)
 49                         : $sock;

Any particular reason here, for the avoidance of ->configure() if there
are no arguments? I was relying on my configure to pass down the default
Protocol / Domain / etc...

-- 

Paul Evans

From gbarr@pobox.com on 2010-01-15 19:51:25:

On Jan 15, 2010, at 11:10 AM, Paul Evans via RT wrote:

>       Queue: IO
> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=53606 >
> 
> On Mon Jan 11 15:37:20 2010, gbarr@pobox.com wrote:
>> Any subclass of IO::Socket should define its own configure, otherwise
>> you will end up
>> with a loop due to the line that follows the bless
>> 
>>    bless($sock, $domain2pkg[$domain]);
>>    $sock->configure($arg);
> ...
>> IO::Socket::Netlink needs to define a configure method, even if it
>> does nothing
> 
> It does contain one of these. That doesn't help. See below.

Why does it not help ? what does your configure look like ?

It should not be calling SUPER::configure, if it does, then it will
create a call loop.

Every subclass need to define its own configure, IO::Socket::configure
should ONLY be called for IO::Socket->new(Domain => 'foo'); so the
object gets re-blessed and then the configure of the right subclass
is called

If you are calling IO::Socket::Netlink->new then IO::Socket::configure
should *NEVER* be called at all.

Graham.

From gbarr@pobox.com on 2010-01-15 19:54:05:

On Jan 15, 2010, at 11:34 AM, Paul Evans via RT wrote:

>       Queue: IO
> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=53606 >
> 
> Actually, scrub all that. I've just realised the configure() method
> doesn't get called on subclasses anyway.
> 
> Instead I was relying on the fact that my configure() would get called for
> 
>  my $sock = IO::Socket::Netlink::Taskstats->new;
> 
> when in fact it doesn't, because there are no arguments:
> 
> 48     return scalar(%arg) ? $sock->configure(\%arg)
> 49                         : $sock;
> 
> Any particular reason here, for the avoidance of ->configure() if there
> are no arguments?

configure should only be called once. this is to allow manual calling of
configure

$s = IO::Socket->new;
$s->configure(...);

>  I was relying on my configure to pass down the default
> Protocol / Domain / etc...

what defaults ?

Graham.
toddr commented 6 years ago

@leonerd: This ticket seems to have stalled. Is there something you suggest actioning out of this case?