The error occurs because you are calling tempdir before your loop. This makes File::Temp call srand internally, and all subsequent forks inherit the seeded random value.You can 'fix' this behaviour by explicitly calling srand($$) before your call to File::Temp->new() in the loop. You're probably right that it would be a good idea for the documentation to point out this particular quirk (or the module could work around it by comparing $$ between rand calls calling srand explicitly), I suggest you file a bug report at rt.cpan.org.

As an aside, you would probably have gotten a quicker response or even have found out the reason for yourself if the code you posted had been reduced to the bits necessary to demonstrate this bug. This would have sufficed (with the fix commented out):

use strict;
use warnings;
use Parallel::ForkManager;
use File::Temp qw(tempdir tempfile);

my $pm=new Parallel::ForkManager(15);
my $tempdir = tempdir();

for my $i (1..15) {
      $pm->start and next;
#     srand($$);
      my $fh = File::Temp->new(TEMPLATE => "sappyXXXXXXXX",
                               UNLINK => 0) or die "Could not make tem
+pfile: $!";
      close $fh or die "Could not close tempfile: $!";
      $pm->finish();
}
$pm->wait_all_children;
[download]

The error occurs because you are calling tempdir before your loop. This makes File::Temp call srand internally, and all subsequent forks inherit the seeded random value.

Thanks for this! I did not know this is how things work, didn't bother looking through the source for more than a few minutes.

FWIW, I call tempdir first because I want all the forks to dump their files into the same dir so I can collect their output easily in the parent when they finish. (I would think this would be a common pattern, but maybe not.)

This is why I included slightly more source than you personally would prefer -- I wanted to show a bit of why I am doing this the way I am doing it. It is reduced considerably from the actual code (down to 26 lines not counting __DATA__ vs. 13 lines for yours). On further consideration, I would probably cut it down as you suggest, and deal with any "why did you do this" questions as they come up (instead of preempting them).

In any case, I doubt I would have stumbled on anything involving srand no matter how much I cut down my code, as it was never in my code! I only know about it because you pointed it out to me, so thanks.

RT

That's not necessarily what I meant. If you'd have cut down your code to the essentials you would have removed the tempdir call before the loop and suddenly it would not have failed. You'd then at least have this extra bit of information which you could have checked up on yourself in the File::Temp code or added to the post. And it would have been easier for someone to look at the problem and figure out what the problem was without the irrelevant bits of information. As I said, just an aside for future reference, sorry if it maybe came across a little harsh.


Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian W. Kernighan

NamedProcess.20051129.110401.1234.ext

Millions of systems the world over have used this type of method (first mainly in C and now also in Perl) for decades and it can safely be called a de facto standard to do so. By comparison, using a CPAN module makes your system less supportable and maintainable. Note also that most systems will need their own date and time formatting to match the above formats anyway and this reduces the unique tmpfile subroutine to a trivial one-liner:

    return join( '.', shift(), ProjectDate(), ProjectTime(), $$, shift
+() );
[download]

Your method will fail to generate a unique name if you are creating more than one file per process per second (which can happen easy enough), or even if you're on a system that does not handle PIDs sanely. File::Temp exists exactly because it is very easy to fall into this kind of trap and surprisingly hard to avoid all traps that come with temporary file generation. "Millions of systems the world over" do this badly and sometimes fail because their author did not consider all corner cases (especially since such errors tend to show up only on loaded production systems and not during testing). Using File::Temp for temporary files is definitely a best practice in my book and there would have to be very specific reasons for recommending against it.


Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian W. Kernighan

File::Temp exists exactly because it is ... surprisingly hard to avoid all traps that come with temporary file generation.

Thanks, this was my thinking precisely. I have some code I use to generate unique IDs using $$ and various other vars, which would have worked fine, but I figured it was about time I learn File::Temp. Also, I'm lazy ;->

You're probably right. Re-rolling a solution is probably a good idea. Especially since the module is a core module...that probably means it doesn't work that well. By your reasoning, we should eschew any module that we didn't write ourselves. Best of luck in that endeavor.

thor

The only easy day was yesterday </tt>

If a task is simple enough, it is usually less work to do it yourself.

Sincere thanks for the perspective, but my milage varies here. I likely have less experience than you, so my default is almost always to reach for the nearest CPAN solution. It saves me huge amounts of time, and when it doesn't, I am sometimes able to send a bugfix or documentation patch upstream to improve the module.

In the vast majority of cases, using CPAN is a win, at least for me.