Explicitly close some file handles #201
Conversation
|
Can you explain to me why this is necessary, with exception of $wfh in one of the files, most of these look like the scope should end the filehandle anyway, so they seem unnecessary. Am I wrong about when filehandles are cleaed up, is it at sub exit instead of scope exit? |
|
@exodist IMO the fh should also close when exiting the scope. So yes most of them are not necessary. I agree some close are not necessary, but I also realized that the existing code was doing a pretty good job of using explicit close when needed, so I decided to add them too. |
|
ah, that makes sense. Did this patch solve your "too many handles" issue? |
|
I'm currently working on this right now, the answer is not obvious as this is a flapping issue... that we start noticing since the update... so might need a few extra days to confirm if it fixes it or not |
|
ok, either way I have no issues with this patch, I think it should be a no-op, explicit close is fine though too. But if you are still working on the problem I will keep it open for any additional patches you may want to attach. |
|
this is not good enough, I can still see some random issues while running our test suite |
|
I wonder if there's a refcount issue introduced recently that might be the source of the problem. is something keeping track of dead handles in a way that's maybe not getting freed? FTR we have about 4000 test files and are running at -j12 |
|
just got: |
toddr
changed the title
|
Actually the problem happened on 1.000000. So this may go back to a problem between 0.999009 and 1.000000 |
|
Are open files limited by user, process, or system? |
|
On most Linux systems, the default for compiled binaries is a max of 1024 file handles per process. |
|
Don't worry about this for now. I have a way to find the root cause on monday. it should be easy to detect. I just need to check |
|
I assume if I can tell you what files aren't closing, then it'll be easy to determine the root cause? |
|
Potentially. I would focus on the collector and the runner, they are the ones that need the most filehandles. The runner creates the handles for the children when it spawns them, but should let them go right after. The collector opens all the files produced by tests, but should let them go when a test completes. My hunch is that either the collector is not letting go of file handles when it is done with them (unexpected references kept?), Or with 12 processes running there is a combination of 12 of your tests that combined with what yath uses result in too many files. Maybe even 1 or 2 filehandle heavy tests? |
|
Thanks @exodist for these details. We could limit the number of jobs opened to avoid(/fix) the issue Making sure |
|
The real fix is #203, this is just good to have |
| @@ -33,6 +33,7 @@ BEGIN { | |||
| if ($no_stat{lc($^O)}) { | |||
| opendir(my $dh, $dir) or die "$!"; | |||
| my $key = join ':' => sort readdir($dh); | |||
| closedir($dh); | |||
| @@ -318,6 +318,7 @@ sub find_libraries { | |||
| next unless -f $path && $path =~ m/\.pm$/; | |||
| push @found => [$path, $prefix]; | |||
| } | |||
| closedir($dh); | |||
| @@ -129,6 +129,7 @@ sub yath { | |||
| push @lines => @new; | |||
| print map { chomp($_); "DEBUG: > $_\n" } @new if $debug > 1; | |||
| } | |||
| close($rh); | |||
| @@ -374,6 +374,9 @@ sub update_io { | |||
| my $msg = "$_[0] at $caller[1] line $caller[2] ($caller2[1] line $caller2[2]).\n"; | |||
| print $stderr $msg; | |||
| print STDERR $msg; | |||
| close($out_fh); | |||
| close($err_fh); | |||
| close($in_fh) if $in_fh; | |||
After updating to v1.000038 from v1.000000 we can start noticing some
Too many open filesissues.I did a quick review of the change from
v1.000000..v1.000038and check all new introduced~openand add an extra explicit close for them