bpo-30264: ExpatParser closes the source on error

[vstinner]

ExpatParser.parse() of xml.sax.xmlreader now always closes the
source: close the file object or the urllib object if source is a
string (not an open file-like object). The change fixes a
ResourceWarning on parsing error.

Add test_parse_close_source() unit test.

[mention-bot]

@Haypo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bitdancer, @serhiy-storchaka and @loewis to be potential reviewers.

[codecov]

Codecov Report

Merging #1451 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1451      +/-   ##
==========================================
+ Coverage   83.44%   83.44%   +<.01%     
==========================================
  Files        1368     1368              
  Lines      346460   346477      +17     
==========================================
+ Hits       289099   289130      +31     
+ Misses      57361    57347      -14

Impacted Files	Coverage Δ
Lib/test/test_sax.py	97.66% <100%> (+0.02%)	⬆️
Lib/xml/sax/expatreader.py	65.55% <100%> (+0.82%)	⬆️
Lib/wsgiref/handlers.py	88.41% <0%> (-0.86%)	⬇️
Lib/pydoc.py	62% <0%> (+0.06%)	⬆️
Lib/threading.py	82.17% <0%> (+0.17%)	⬆️
Lib/test/test_buffer.py	96.71% <0%> (+0.17%)	⬆️
Lib/test/test_random.py	98.65% <0%> (+0.19%)	⬆️
Lib/test/lock_tests.py	86.66% <0%> (+0.28%)	⬆️
Lib/heapq.py	97.32% <0%> (+2.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76a3e51...ed9b102. Read the comment docs.

[gibfahn]

Sorry if this is the wrong place to post this.

We hit some issues with this change (which could be due to my own misunderstandings, let me know if that's the case!)

My questions are:

1. Isn't this a breaking change? It certainly breaks code we're using in production.
2. Why is the sax parser closing file descriptors that it didn't open? Cleaning up your own resources I understand, but cleaning up a fd that was passed to you seems odd.
3. What do you do if you need access to the file descriptor after parsing it (because you parse it in place)?

For file descriptors that point to files on disk we can work around it by reopening the file, but for something like a StringIO buffer (see simplified example below) I'm not aware of any way to get around the problem.

import xml.sax
import StringIO

# Some StringIO buffer.
source = StringIO.StringIO(b'<_/>')

# Do some parsing.
xml.sax.parse(source, xml.sax.handler.ContentHandler())

# Try to do some other parsing (fails).
xml.sax.parse(source, xml.sax.handler.ContentHandler())

EDIT: This should probably have been on #1476

[serhiy-storchaka]

The issue on the Python bug tracker is better place: https://bugs.python.org/issue30264.

GitHub is only for discussing technical details of a concrete PR. This PR was merged a year ago.

[gibfahn]

Thanks @serhiy-storchaka , raised https://bugs.python.org/issue33732