Fix windows file descriptor leak #4023
Conversation
Gallaecio
force-pushed
the
fix-windows-file-descriptor-leak
branch
from
4b8dfc9 to
b3c0141
Compare
Gallaecio
changed the title
Codecov Report
@@ Coverage Diff @@
## master #4023 +/- ##
=======================================
Coverage 85.52% 85.52%
=======================================
Files 167 167
Lines 9741 9741
Branches 1461 1461
=======================================
Hits 8331 8331
Misses 1153 1153
Partials 257 257
|
| @@ -242,7 +242,7 @@ def open_spider(self, spider): | |||
| def close_spider(self, spider): | |||
| slot = self.slot | |||
| if not slot.itemcount and not self.store_empty: | |||
| return | |||
| return defer.maybeDeferred(slot.storage.store, slot.file) | |||
There was a problem hiding this comment.
Could you please explain how this fix works?
There was a problem hiding this comment.
In FeedExporter.open_spider we are opening the output file, regardless of the value of self.store_empty. slot.storage.store is responsible for closing that file; see scrapy.extensions.feedexport.FileFeedStorage.store.
The code here, however, was skipping that step when there was no item to store and storing an empty file was disabled. And that was a problem on Windows.
There was a problem hiding this comment.
Aha, this makes sense. Could you please add a small comment here though? Now I see that this is a copy-paste of the code below (d = defer.maybeDeferred(slot.storage.store, slot.file)). But it is not obvious, from these few lines themselves, that slot.storage.store closes a file.
Fixes #3391