IO::Poll relies on stable stringification of IO handles [rt.cpan.org #93107]

[toddr]

Migrated from rt.cpan.org#93107 (status was 'new')

Requestors:

• [email protected]

Attachments:

• rt93107.patch

From [email protected] on 2014-02-18 01:51:18:

IO::Poll uses IO objects as keys in internal hashes, relying therefore on the stringification of these IO handles to be stable.

The use of IO::Socket::SSL and late-upgrade after creation breaks this logic. For better or worse, IO::Socket::SSL->SSL_upgrade performs a rebless operation on the IO handle object, thus changing its stringification.

This bug manifests itself when using SSL upgrades on IO::Poll-multiplexed sockets, causing a 100% CPU spin the first time a socket is closed. The different stringification of the object handle means that $poll->remove() doesn't successfully remove it, causing the underlying poll(2) call to the kernel to still request a now-invalid file descriptor, causing it to return instantly with POLLNVAL.

I would suggest instead all uses of {$io} as a hash key be replaced with {refaddr $io}, because the refaddr of an object /is/ guaranteed inviolate during its lifetime.

-- 

Paul Evans

From [email protected] on 2014-02-18 01:55:35:

On Mon Feb 17 20:51:18 2014, PEVANS wrote:
> I would suggest instead all uses of {$io} as a hash key be replaced
> with {refaddr $io}, because the refaddr of an object /is/ guaranteed
> inviolate during its lifetime.

As a side benefit, this would also defend against IO handles with stringification overload magic, which would otherwise either fail to identify, or would cause identity clashes if they overlapped.

-- 

Paul Evans

From [email protected] on 2014-02-20 18:02:29:

On Mon Feb 17 20:51:18 2014, PEVANS wrote:
> I would suggest instead all uses of {$io} as a hash key be replaced
> with {refaddr $io}, because the refaddr of an object /is/ guaranteed
> inviolate during its lifetime.

Bugfix + regression test attached.

-- 

Paul Evans

[toddr]

@lenoerd submitted pull request for testing.

[Leont]

This sounds like hell once threading gets involved

[toddr]

I had to fix several things in the tests to be backward compatible. The big problem, though is that this seems to fall on its face on windows.

[toddr]

@leonerd should this be pursued or closed?

[leonerd]

The problem is still a problem, even if my original suggested fix doesn't work in all cases. We should keep it open awaiting a better fix.

[Leont]

I had to fix several things in the tests to be backward compatible. The big problem, though is that this seems to fall on its face on windows.

My guess, without having looked at the code, is that this is because of pseudo-threads: in different threads the handles would have different addresses.

If so, the solution may be to use a fieldhash (on 5.10+), or something based on its idea of identities.

[toddr]

The code is here Dual-Life/IO@b1ad595