Skip Menu |
 

This queue is for tickets about the Net-Daemon CPAN distribution.

Report information
The Basics
Id: 55512
Status: resolved
Worked: 6 hours (360 min)
Priority: 0/
Queue: Net-Daemon

People
Owner: TODDR [...] cpan.org
Requestors: mcmahon [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.43
Fixed in: (no value)

Attachments


Subject: Patch for 5.10 failures (closes 39759, 50300, 53833)
Download (untitled) / with headers
text/plain 273b
The attached patch fixes the failing tests in 0.43 under 5.10.1. The problem is that the code uses the old thread model; the attached code checks for 5.10 or better, loads the appropriate thread modules, and shares the variables that throw the errors. Test suite passes.
Subject: Net-Daemon-5.10-support.patch
diff -ruad Net-Daemon/ChangeLog Net-Daemon-fixed/ChangeLog --- Net-Daemon/ChangeLog 2010-03-12 14:38:33.000000000 -0800 +++ Net-Daemon-fixed/ChangeLog 2010-03-12 14:45:56.000000000 -0800 @@ -1,4 +1,7 @@ - +2009-03-12 Joe McMahon <mcmahon@blekko.com> (0.44) + * Added necessary thread sharing to work with 5.10 + threads model: regexp-threads, + * Bumped minimum required perl to 5.10. 2007-06-17 Malcolm Nooning <m.nooning@comcast.net> (0.43) * lib/Net/Daemon.pm Needed to up the VERSION number 2007-06-16 Malcolm Nooning <m.nooning@comcast.net> (0.42) diff -ruad Net-Daemon/lib/Net/Daemon.pm Net-Daemon-fixed/lib/Net/Daemon.pm --- Net-Daemon/lib/Net/Daemon.pm 2010-03-12 14:38:33.000000000 -0800 +++ Net-Daemon-fixed/lib/Net/Daemon.pm 2010-03-12 15:13:14.000000000 -0800 @@ -20,7 +20,6 @@ # ############################################################################ -require 5.004; use strict; use Getopt::Long (); @@ -30,10 +29,21 @@ use Net::Daemon::Log (); use POSIX (); - package Net::Daemon; -$Net::Daemon::VERSION = '0.43'; +# Dummy share() in case we're not 5.10. If we are, require/import of +# threads::shared will replace it appropriately. +my $this_is_510 = $^V ge v5.10.0; +if ($this_is_510) { + no warnings 'redefine'; + eval { require threads; }; + eval { require threads::shared; }; +} +else { + eval { require Threads; }; +} + +$Net::Daemon::VERSION = '0.43'; @Net::Daemon::ISA = qw(Net::Daemon::Log); # @@ -41,6 +51,7 @@ # regexp-threads.) # $Net::Daemon::RegExpLock = 1; +threads::shared::share(\$Net::Daemon::RegExpLock) if $this_is_510; use vars qw($exit); diff -ruad Net-Daemon/log Net-Daemon-fixed/log --- Net-Daemon/log 2010-03-12 14:38:33.000000000 -0800 +++ Net-Daemon-fixed/log 2010-03-12 15:13:32.000000000 -0800 @@ -1,10 +1,10 @@ -ok 3 -ok 2 ok 1 +ok 3 ok 6 -ok 4 +ok 2 ok 5 -ok 7 -ok 9 ok 10 +ok 9 +ok 4 +ok 7 ok 8 diff -ruad Net-Daemon/regexp-threads Net-Daemon-fixed/regexp-threads --- Net-Daemon/regexp-threads 2010-03-12 14:38:33.000000000 -0800 +++ Net-Daemon-fixed/regexp-threads 2010-03-12 15:11:30.000000000 -0800 @@ -13,9 +13,18 @@ # # +my $this_is_510 = $^V ge v5.10.0; + use Thread (); +if ($this_is_510) { + eval {require threads::shared}; +} my $numChilds; +if ($this_is_510) { + eval { threads::shared::share($numChilds) }; +} + my $regExpLock = @ARGV ? 1 : 0; # Repeat generating a random number and check if it contains the diff -ruad Net-Daemon/t/threadm.t Net-Daemon-fixed/t/threadm.t --- Net-Daemon/t/threadm.t 2010-03-12 14:38:33.000000000 -0800 +++ Net-Daemon-fixed/t/threadm.t 2010-03-12 14:47:19.000000000 -0800 @@ -24,7 +24,6 @@ exit 0; } - my($handle, $port); if (@ARGV) { $port = shift @ARGV;
From: m.nooning [...] comcast.net
Download (untitled) / with headers
text/plain 620b
Hello Joe, First of all, thank you very much for your patch! It appears that the patch for t/threadm.t got cut off. It ended with the two lines below. my($handle, $port); if (@ARGV) { $port = shift @ARGV; For one, it introduces a '{' without a corresponding '}', so I fear I do not have the entire patch. For another, the line below is missing. = Net::Daemon::Test->Child ($numTests, $^X, 't/server', '--timeout', 20, '--mode=threads'); Can you please comment on this? Also, I have a Windows and a Linux box to test this on. What machine(s) do you have to test on? Thanks again for doing this.
RT-Send-CC: m.nooning [...] comcast.net
Download (untitled) / with headers
text/plain 163b
I'll need to look into this item further. The patch has made perl 5.10 the minimum version. If that's really required, we'll need to look at other possible fixes.
Download (untitled) / with headers
text/plain 181b
Sorry, haven't been monitoring. I'll repost that tomorrow. I think it may be possible to check the Perl version and leave it still at 5.8. If I can, I'll try that tomorrow as well.
From: m.nooning [...] comcast.net
Download (untitled) / with headers
text/plain 999b
My previous reply still holds. In addition, I have been looking into the reason the threads tests no longer work. The thread model used may be the one that was removed from Perl as of version 5.10. If so, the threads portion of the Net::Daemon module will have to be rewritten to support the new version of threads. "If" statements would have to be put in to differentiate between the threads model/modules used, based on the Perl version. Unfortunately, I do not know threads programming, and I do not have plans to learn it. Until we get a more knowledgeable person to look into this, the t/thread.t tests will fail for Perl > 5.10. I will be updating the Net::Daemon cpan site to warn users. I will continue to allow the tests to fail, as I think it is better to let users of the new Perl versions know right away that they can no longer rely on Net::Daemon if they need to use threads. Eventually, this module will have to be updated or deprecated. Anyone is welcome to add comments.
Subject: Re: [rt.cpan.org #55512] Patch for 5.10 failures (closes 39759, 50300, 53833)
Date: Sun, 18 Jul 2010 02:54:37 -0400
To: bug-Net-Daemon [...] rt.cpan.org
From: "Daniel Macks" <dmacks [...] netspace.org>
Download (untitled) / with headers
text/plain 616b
The patcher and the patch assert that 5.10 is a formal requirement, but I don't see where in the patch it's actually necessary to bump that high. The adjustments to the module code appear protected by "if ($^V ge v5.10.0)" with alternate adjustments (or fallback to existing module code) on lower platforms. And that's the ideal behavior: add new calling semantics or other workarounds on platforms that fail or otherwise need them and leave the platforms where the existing module works unchanged (rather than *replacing* the works-on-old with needed-on-new code).   dan   -- Daniel Macks dmacks@netspace.org
Download (untitled) / with headers
text/plain 760b
On Sun Jul 18 02:54:46 2010, dmacks@netspace.org wrote: Show quoted text
> The patcher and the patch assert that 5.10 is a formal requirement, but > I don't see where in the patch it's actually necessary to bump that > high. The adjustments to the module code appear protected by "if ($^V > ge v5.10.0)" with alternate adjustments (or fallback to existing module > code) on lower platforms. And that's the ideal behavior: add new > calling semantics or other workarounds on platforms that fail or > otherwise need them and leave the platforms where the existing module > works unchanged (rather than *replacing* the works-on-old with > needed-on-new code).
Yep, you're right. I simply forgot to reset the required version after testing with 5.10 to make sure the patch worked.
RT-Send-CC: m.nooning [...] comcast.net, dmacks [...] netspace.org
Download (untitled) / with headers
text/plain 206b
I retried this on Windows, Fedora Linux from a few months ago, then updated the Fedora and it still passes, so it looks like version 0.46 is a winner. Again, thank you very, very much for all of your help.


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.