Only cache io_socket_proto if protocol is truthy #17157
Comments
From afresh1@openbsd.orgCreated by andrew@afresh1.comWhen OpenBSD implemented support for SO_PROTOCOL to getsockopt(2)[1], I had trouble deciding on whether to actually just remove the entire In either case, this causes this test to pass because $ perl dist/IO/t/cachepropagate-unix.t This test program on OpenBSD-current prints without the attached patch: guess: 0 --- BEGIN testprogram.pl --- #!/usr/bin/perl; use IO::Socket::UNIX; use File::Temp qw< tempdir >; my $socketpath = tempdir( CLEANUP => 1 ) . '/testsock'; my $old = IO::Socket::UNIX->new(Type => SOCK_DGRAM, Local => $socketpath); unlink($socketpath); --- END testprogram.pl --- Perl Info |
From afresh1@openbsd.org0001-Only-cache-io_socket_proto-if-protocol-is-truthy.patchFrom fb7c0942ed53c49d3bbe8d153622055b3433fb65 Mon Sep 17 00:00:00 2001
From: Andrew Hewus Fresh <afresh1@openbsd.org>
Date: Sun, 22 Sep 2019 17:36:47 -0700
Subject: [PATCH] Only cache io_socket_proto if protocol is truthy
According to socket(2):
A value of 0 for protocol will let the system select an appropriate
protocol for the requested socket type.
While linux, and possibly others, return 0 for this, OpenBSD recently
got support for SO_PROTOCOL in getsockopt(2) and instead of returning
0, indicating it will choose the correct protocol, it instead returns
the protocol that was chosen. That means caching "0" as the chosen
protocol means that what $sock->protocol returns is not the same as what
getsockopt($sock, SOL_SOCKET, SO_PROTOCOL) returns.
While there is ongoing discussion about whether returning 0 vs something
else is correct, the current situation shows that this value is
implementation specific and we should not cache something that might not
be right.
---
dist/IO/lib/IO/Socket.pm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/dist/IO/lib/IO/Socket.pm b/dist/IO/lib/IO/Socket.pm
index 28fa1ec149..0d93575ccf 100644
--- a/dist/IO/lib/IO/Socket.pm
+++ b/dist/IO/lib/IO/Socket.pm
@@ -82,7 +82,12 @@ sub socket {
${*$sock}{'io_socket_domain'} = $domain;
${*$sock}{'io_socket_type'} = $type;
- ${*$sock}{'io_socket_proto'} = $protocol;
+
+ # "A value of 0 for protocol will let the system select an
+ # appropriate protocol"
+ # so we need to look up what the system selected,
+ # not cache PF_UNSPEC.
+ ${*$sock}{'io_socket_proto'} = $protocol if $protocol;
$sock;
}
--
2.22.0
|
From @jkeenanOn Mon, 23 Sep 2019 01:03:12 GMT, afresh1@openbsd.org wrote:
Applied patch attached to a branch for smoke-testing: Thank you very much. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Mon, 23 Sep 2019 01:58:25 GMT, jkeenan wrote:
With the patch the branch is experiencing failures in dist/IO/t/cachepropagate-unix.t on all our NetBSD smoke-testing rigs. It's experiencing other failures on one of them (http://perl5.test-smoke.org/report/95354 but ignore the failures in syslog.t). Thank you very much. -- |
From @jkeenanOn Mon, 23 Sep 2019 11:45:26 GMT, jkeenan wrote:
In the preceding smoke-test report, most of the failures may be resource constraints. However, here's what I got for the test in question: ##### ok 1 - stream socket created # Failed test 'protocol defined' # Failed test 'protocol defined' Test Summary Report ../dist/IO/t/cachepropagate-unix.t (Wstat: 512 Tests: 15 Failed: 2) -- |
From andrew@afresh1.comOn Mon, Sep 23, 2019 at 04:53:57AM -0700, James E Keenan via RT wrote:
<SNIP>
This is kinda what I expected, not sure what the correct solution is. It would be easy to add in some `skip` calls if `$listener->protocol` I'm not sure how to handle the fact that `->protocol` can return either
-- Speed matters. |
I'm sure there's a better fix, but at the moment I don't know what it is. Perl/perl5#17157
|
Fixed by 6212a44 and a few other follow-up commits. |
Migrated from rt.perl.org#134446 (status was 'open')
Searchable as RT134446$
The text was updated successfully, but these errors were encountered: