[DataPipe] Properly cleanup unclosed files within generator function

[ejguan]

There is a case that file.close is never called because when generator function has never reached to the end. A simple example would be zip two datepipes with different length. The longer DataPipe would never reach the end of generator and then it will be cleaned up by gc. So, the line of file.close is not executed. (This is the reason that Vitaly has to create this hack to retrieve all remaining data to make sure generator function is fully executed)

However, this hack introduces another problem where an infinite datapipe would make zip never end as it would try to deplete the infinite iterator. See: pytorch/data#865

So, in this PR, I am adding a try-finally clause to make sure the file.close is always executed during the destruction of generator object. Then, we don't need the hack within zip any more.

• [3/4] Properly cleanup unclosed files within generator function data#910
• [1/4][DataPipe] Properly cleanup unclosed files within generator function pytorch#89973
• [4/4][DataPipe] Remove iterator depletion in Zipper pytorch#89974

cc @pmeier @bjuncek

[pmeier]

SGTM, thanks @ejguan! One question out of curiosity:

I found a corner case that file.close is never called because when generator function has never reached to the end. A simple example would be zip two datepipes with different length. The longer DataPipe would never reach the end of generator and then it will be cleaned up by gc. So, the line of file.close is not executed.

Is this a problem for our datasets? Because that would indicate that something is off in our setup. If we zip, the datapipes should always have the same length. If not, did you figure out the issue for the datasets you fixed?

[ejguan]

Is this a problem for our datasets? Because that would indicate that something is off in our setup. If we zip, the datapipes should always have the same length. If not, did you figure out the issue for the datasets you fixed?

@pmeier
The datasets are fine except the modified generator functions in this PR. And, torchvision would be broken is caused by demux that I will change in pytorch/pytorch#89973
The buffer within demux should be clean up when generator function exits.

[pmeier]

The datasets are fine except the modified generator functions in this PR

How are they broken? Are we zipping two pipes of different length like in the example you gave? Because that likely is not intended and thus we should not just mitigate, but investigate why this happens.

And, torchvision would be broken is caused by demux that I will change in pytorch/pytorch#89973
The buffer within demux should be clean up when generator function exits.

Not sure I understand. Are you saying that with this PR (3/4) here, our datasets will be broken after the core PR (2/4) is merged?

[ejguan]

How are they broken? Are we zipping two pipes of different length like in the example you gave? Because that likely is not intended and thus we should not just mitigate, but investigate why this happens.

That makes sense. I have validated those datapipes have the same lengths. Changing zip would cause Error for the dataset in TorchVision due to internal mechanism of zip. Let's say we have two datapipes with the same length and we want to zip them together. zip would stop when the first iterator reaches StopIteration even though the second datapipe is about to reach Stopiteration. Then, the second datapipe's iterator would never execute file.close() as the iterator has not been iterated to the end.

Not sure I understand. Are you saying that with this PR (3/4) here, our datasets will be broken after the core PR (2/4) is merged?

I am sorry that I should be clear that this PR is a blocker for pytorch/pytorch#89974. It's totally fine as long this PR is landed before pytorch/pytorch#89974..

[NivekT]

LGTM!

[pmeier]

Makes sense, thanks Erjia!

[ejguan]

@pmeier Please feel free to merge this PR.

[github-actions]

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py