Not OK: perl v5.7.3 +DEVEL16055 on cygwin-64int 1.3.9(0.5132)

p5pRT commented 22 years ago

Migrated from rt.perl.org#8978 (status was 'resolved')

Searchable as RT8978$

p5pRT commented 22 years ago

From @ysth

This is a build failure report for perl from sthoenna@efn.org\, generated with the help of perlbug 1.33 running under perl v5.7.3.

ext/PerlIO/t/via.t is hanging after the last test.

Looked at it and saw the magic "1 while unlink". This loops forever on open files on cygwin. (It defers the unlink and returns true each time you call unlink.)

Problem is\, the test is closing the file after each successful open\, so the "1 while unlink" should be safe.

The last two open tests which are supposed to have open fail are in fact opening the file\, even though open fails and $fh is left alone (Dump of *$fh{IO} shows no file pointers set).

No idea how to fix this\, but this changes the endless loop into a failing test:

Inline Patch

```diff --- ext/PerlIO/t/via.t.orig Sun Apr 7 09:19:54 2002 +++ ext/PerlIO/t/via.t Sun Apr 21 19:18:22 2002 @@ -14,7 +14,7 @@ my $tmp = "via$$"; -use Test::More tests => 11; +use Test::More tests => 12; my $fh; my $a = join("", map { chr } 0..255) x 10; @@ -45,8 +45,8 @@ no warnings 'layer'; ok( ! open($fh,">Via(Unknown::Module)", $tmp), 'open Via Unknown::Module will fail'); is( $warnings, "", "don't warn about unknown package" ); -} - -END { - 1 while unlink $tmp; } + +my $i; +1 while ++$i < 100 && unlink $tmp; +ok ( ! unlink($tmp), "don't leave $tmp open" ); ```

End of Patch.

Flags: category=install severity=none

Site configuration information for perl v5.7.3:

Configured by sthoenna at Sun Apr 21 17:12:41 2002.

Summary of my perl5 (revision 5.0 version 7 subversion 3 patch 16055) configuration: Platform: osname=cygwin\, osvers=1.3.9(0.5132)\, archname=cygwin-64int uname='cygwin_98-4.10 yo 1.3.9(0.5132) 2002-01-21 12:48 i686 unknown ' config_args='-de -Dusedevel -Duse64bitint -Doptimize=-g' hint=recommended\, useposix=true\, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=undef usemymalloc=y\, bincompat5005=undef Compiler: cc='gcc'\, ccflags ='-DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -I/usr/local/include'\, optimize='-g'\, cppflags='-DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -I/usr/local/include' ccversion=''\, gccversion='2.95.3-5 (cygwin special)'\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=12345678 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12 ivtype='long long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=4 alignbytes=8\, prototype=define Linker and Libraries: ld='ld2'\, ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /usr/lib /lib libs=-lgdbm -lcrypt -lutil perllibs=-lcrypt -lutil libc=/usr/lib/libc.a\, so=dll\, useshrplib=true\, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=dll\, d_dlsymun=undef\, ccdlflags=' ' cccdlflags=' '\, lddlflags=' -L/usr/local/lib'

Locally applied patches: DEVEL16055

@INC for perl v5.7.3: lib /usr/local/lib/perl5/5.7.3/cygwin-64int /usr/local/lib/perl5/5.7.3 /usr/local/lib/perl5/site_perl/5.7.3/cygwin-64int /usr/local/lib/perl5/site_perl/5.7.3 /usr/local/lib/perl5/site_perl .

Environment for perl v5.7.3: HOME=/cygdrive/c/home/sthoenna LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/bin:/cygdrive/c/home/sthoenna/bin:/cygdrive/c/PROGRA~1/YARNW:/usr/bin:/USR/LOCAL/EMACS/EMACS-20.7/BIN:/cygdrive/c/WINDOWS:/cygdrive/c/WINDOWS/COMMAND PERL_BADLANG (unset) SHELL (unset)

p5pRT commented 22 years ago

From @nwc10

On Sun\, Apr 21\, 2002 at 07:55:54PM -0700\, sthoenna@efn.org wrote:

This is a build failure report for perl from sthoenna@efn.org\, generated with the help of perlbug 1.33 running under perl v5.7.3.

ext/PerlIO/t/via.t is hanging after the last test.

Looked at it and saw the magic "1 while unlink". This loops forever on open files on cygwin. (It defers the unlink and returns true each time you call unlink.)

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

Problem is\, the test is closing the file after each successful open\, so the "1 while unlink" should be safe.

Ah. the cygwin/VMS fight.

Erk. pod/perlport.pod says

Don't assume that a single C\ completely gets rid of the file: some filesystems (most notably the ones in VMS) have versioned filesystems\, and unlink() removes only the most recent one (it doesn't remove all the versions because by default the native tools on those platforms remove just the most recent version\, too). The portable idiom to remove all the versions of a file is
1 while unlink "file";

Isn't this safer?

unlink "file" or die "$!" while -f "file";

Although that still doesn't counter the problem of infinite loop if unlink is lying to be helpful.

[But it ought to solve the other lie where things try to stat "file"\, find it missing and "helpfully" fall back to stat "file.exe"]

If this sounds like a hobbyhorse rant on my part\, that's probably because it is :-(

Nicholas Clark

p5pRT commented 22 years ago

From @nwc10

On Mon\, Apr 22\, 2002 at 11:02:00AM +0100\, Nicholas Clark wrote:

Erk. pod/perlport.pod says
    1 while unlink "file";
Isn't this safer?

unlink "file" or die "$!" while -f "file";

ENOCAFFINE

I think I'm talking bollo.cx. I'll shut up until I believe I'm lucid. I'm not convinced that a solution exists to the unlink conundrum. :-(

Nicholas Clark

p5pRT commented 22 years ago

From @jhi

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

    1 while unlink "file";
Isn't this safer?

unlink "file" or die "$!" while -f "file";

I think we want through this discussion once (a year ago?) and the end result was that "1 while unlink" is the safest of them all. (Assuming filesystems don't lie to us.)

-- $jhi++; # http://www.iki.fi/jhi/ # There is this special biologist word we use for 'stable'. # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @gsar

On Mon\, 22 Apr 2002 16:33:46 +0300\, Jarkko Hietaniemi wrote:

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

It looks like a PerlIO bug to me. There appears to be a handle leak somewhere\, because even when the open() fails\, the file is in fact opened\, and the handle isn't remembered anywhere that you can access from the perl level\, because calling close() any number of times after that has no effect. In other words\, the following test will consume a large number of handles\, and on windows will eventually not cleanup the temp file because it is still being held open.

my $tmp = "viabug$$"; my $fh; for (1..10000) { # this open should fail\, and not leak a handle like it does open($fh\,">Via(Unknown::Module)"\, $tmp) and die "open unexpectedly succeeded!"; # this close doesn't suffice to work around the leak close $fh; sleep 1; } END { unlink $tmp; }

Sarathy gsar@ActiveState.com

p5pRT commented 22 years ago

From @ysth

In article \20020422111254\.T3549@plum\.flirble\.org\, Nicholas Clark \nick@ccl4\.org wrote:

On Mon\, Apr 22\, 2002 at 11:02:00AM +0100\, Nicholas Clark wrote:
Erk. pod/perlport.pod says
    1 while unlink "file";
Isn't this safer?

unlink "file" or die "$!" while -f "file";
ENOCAFFINE

I think I'm talking bollo.cx. I'll shut up until I believe I'm lucid. I'm not convinced that a solution exists to the unlink conundrum. :-(

(Is bollo.cx a real domain? Thats almost as good as sci.fi or yes.no.)

Yes\, there is. perlport says: Some platforms can't delete or rename files held open by the system. Remember to C\ files when you are done with them. Don't C\ or C\ an open file.

The 1 while unlink syntax is perfectly ok unless the file is open. If the file is open\, it may work\, fail\, or loop endlessly\, depending on os.

But the 1 while unlink is not the problem here. The file *shouldn't* be open. The open($fh\,">Via(Unknown::Module)"\, $tmp) call returns false and leaves $fh closed\, but somewhere in the depths of perlio\, $tmp is opened and left open anyway.

p5pRT commented 22 years ago

From @ysth

In article \20020422163346\.Y15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

No\, I think perlio is broken and this is exposed only on cygwin due to the deferred unlink "feature".

There are two perlio problems here. First\, mode ">Via(Unknown::Module)" is actually opening the file (thus clobbering the contents):

~/bleadperl/perl $cp Changes afile ~/bleadperl/perl $ll afile -rw-r--r-- 1 sthoenna unknown 4094040 Apr 22 09:19 afile ~/bleadperl/perl $./perl -Ilib -wle 'print(open(my $fh\,">Via(Unknown::Module)"\, "afile") ? "opened" : "not opened")' Cannot find package 'Unknown::Module'. not opened ~/bleadperl/perl $ll afile -rw-r--r-- 1 sthoenna unknown 0 Apr 22 09:19 afile

^!!!!

This problem may be one to just document and leave alone. I don't know how difficult it is to fix.

Second problem (which results from the first) is that when perlio can't find Unknown::Module\, the file isn't actually closed (though open's file handle isn't set and open returns false). This seems to me more serious\, since it leaks file handles (as well potentially causing users other trouble).

This patch tests both problems. I expect test 14 (first problem above) should fail everywhere. Test 15 (2nd problem above) should fail on windows/dos. (Test::More really needs an is_nicely that formats !isprints with \x).

Inline Patch

```diff --- ext/PerlIO/t/via.t.orig Sun Apr 7 09:19:54 2002 +++ ext/PerlIO/t/via.t Mon Apr 22 11:13:38 2002 @@ -14,7 +14,7 @@ my $tmp = "via$$"; -use Test::More tests => 11; +use Test::More tests => 15; my $fh; my $a = join("", map { chr } 0..255) x 10; @@ -30,7 +30,7 @@ { local $/; $b = <$fh> } ok( close($fh), "close input file"); -is($a, $b, 'compare original data with filtered version'); +is($b, $a, 'compare original data with filtered version'); { @@ -45,8 +45,14 @@ no warnings 'layer'; ok( ! open($fh,">Via(Unknown::Module)", $tmp), 'open Via Unknown::Module will fail'); is( $warnings, "", "don't warn about unknown package" ); -} - -END { - 1 while unlink $tmp; } + +ok( open($fh," } +ok( close($fh), "close input file"); + +is($b, $a, 'see if >Via(Unknown::Module) clobbered file'); + +my $i; # guard against endless loop if file erroneously left open +1 while ++$i < 100 && unlink $tmp; +ok( !-e $tmp, "remove temp file" ); End of Patch. ```

p5pRT commented 22 years ago

From @jhi

On Mon\, Apr 22\, 2002 at 09:06:52AM -0700\, Yitzchak Scott-Thoennes wrote:

In article \20020422163346\.Y15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

No\, I think perlio is broken and this is exposed only on cygwin due to the deferred unlink "feature".

I agree\, as you can see in the todo list of the latest snapshot.

-- $jhi++; # http://www.iki.fi/jhi/ # There is this special biologist word we use for 'stable'. # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @ysth

In article \20020422213808\.N15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

On Mon\, Apr 22\, 2002 at 09:06:52AM -0700\, Yitzchak Scott-Thoennes wrote:

In article \20020422163346\.Y15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

No\, I think perlio is broken and this is exposed only on cygwin due to the deferred unlink "feature".

I agree\, as you can see in the todo list of the latest snapshot.

I saw the filehandle leak mentioned\, but not the clobbering. So which did you agree to?

Can you apply my second test patch? Or would you prefer a TODO'd version?

p5pRT commented 22 years ago

From @jhi

On Tue\, Apr 23\, 2002 at 04:55:42PM -0700\, Yitzchak Scott-Thoennes wrote:

In article \20020422213808\.N15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

On Mon\, Apr 22\, 2002 at 09:06:52AM -0700\, Yitzchak Scott-Thoennes wrote:

In article \20020422163346\.Y15418@alpha\.hut\.fi\, Jarkko Hietaniemi \jhi@iki\.fi wrote:

This is because the file is open? So doesn't writing the test to be correctly portable by closing the file solve the symptoms?

That's why I was wondering: if closing the file doesn't make cygwin happy\, cygwin sounds like broken.

No\, I think perlio is broken and this is exposed only on cygwin due to the deferred unlink "feature".

I agree\, as you can see in the todo list of the latest snapshot.

I saw the filehandle leak mentioned\, but not the clobbering. So which did you agree to?

I think both are bugs.

Can you apply my second test patch? Or would you prefer a TODO'd version?

TODO'd.

-- $jhi++; # http://www.iki.fi/jhi/ # There is this special biologist word we use for 'stable'. # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @ysth

In article \20020424031436\.P4054@alpha\.hut\.fi\, jhi@iki.fi wrote:

Can you apply my second test patch? Or would you prefer a TODO'd version?

TODO'd.

Inline Patch

```diff --- ext/PerlIO/t/via.t.orig Tue Apr 23 18:18:04 2002 +++ ext/PerlIO/t/via.t Tue Apr 23 18:22:34 2002 @@ -14,7 +14,7 @@ my $tmp = "via$$"; -use Test::More tests => 11; +use Test::More tests => 15; my $fh; my $a = join("", map { chr } 0..255) x 10; @@ -30,7 +30,7 @@ { local $/; $b = <$fh> } ok( close($fh), "close input file"); -is($a, $b, 'compare original data with filtered version'); +is($b, $a, 'compare original data with filtered version'); { @@ -45,8 +45,20 @@ no warnings 'layer'; ok( ! open($fh,">Via(Unknown::Module)", $tmp), 'open Via Unknown::Module will fail'); is( $warnings, "", "don't warn about unknown package" ); -} +} -END { - 1 while unlink $tmp; +ok( open($fh," } +ok( close($fh), "close input file"); + +{ + local $TODO = "Via shouldn't open file with bad module"; + is($b, $a, 'see if >Via(Unknown::Module) clobbered file'); +} + +my $i; # guard against endless loop if file erroneously left open +1 while ++$i < 100 && unlink $tmp; +{ + local $TODO = "Via leaves file open with bad module"; + ok( !-e $tmp, "remove temp file" ); } End of Patch. ```

p5pRT commented 22 years ago

From @ysth

Filehandle leak fixed that was holding file open.

p5pRT commented 22 years ago

@ysth - Status changed from 'open' to 'resolved'

Perl / perl5