Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file-handle resource leaks #9309

Merged
merged 1 commit into from Feb 4, 2018

Conversation

Projects
None yet
2 participants
@bzamecnik
Copy link
Contributor

commented Feb 4, 2018

Issue #9287: Resource leak of file handles

Open resources (ordinary file, HDF5, tar, urlopen response, etc.) were often not closed properly. When an exception is raised the resource might not be closed. This may eg. result in depleting the number of file handles for the process.

Where possible use context managers (with open(...) as f) that close the resource even after catching an exception.

Still, HDF5Matrix, CSVLogger and keras.preprocess.image.load_img() are difficult to fix without changing their API. In those places resource ownership is not properly defined. See comment for HDF5Matrix.

PIL images returned by load_img() should be closed by the user of that function. Possibly using it's context manager:

with keras.preprocess.image.load_img(...) as img:
    # do something with the image
# image closed...

However, I'm not 100% sure if this can be disturbed by operations such as img.convert(), img.resize() done in load_img().

#9287 Fix most of the file-handle resource leaks.
Open resources (ordinary file, HDF5, tar, urlopen response, etc.) were often not
closed properly. When an exception is raised the resource might not be closed.
This may eg. result in depleting the number of file handles for the process.

Where possible use context managers (`with open(...) as f`) that close
the resource even after catching an exception.

Still, HDF5Matrix, CSVLogger and keras.preprocess.image.load_img()
are difficult to fix without changing their API. In those places resource
ownership is not properly defined.

@fchollet fchollet merged commit 4cde148 into keras-team:master Feb 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

ahundt added a commit to ahundt/keras that referenced this pull request Feb 16, 2018

Merge branch 'master' of github.com:fchollet/keras into pr_7508
* 'master' of github.com:fchollet/keras: (57 commits)
  Minor README edit
  Speed up Travis tests (keras-team#9386)
  fix typo (keras-team#9391)
  Fix style issue in docstring
  Prepare 2.1.4 release.
  Fix activity regularizer + model composition test
  Corrected copyright years (keras-team#9375)
  Change default interpolation from nearest to bilinear. (keras-team#8849)
  a capsule cnn on cifar-10 (keras-team#9193)
  Enable us to use sklearn to do cv for functional api (keras-team#9320)
  Add support for stateful metrics. (keras-team#9253)
  The type of list keys was float (keras-team#9324)
  Fix mnist sklearn wrapper example (keras-team#9317)
  keras-team#9287 Fix most of the file-handle resource leaks. (keras-team#9309)
  Pass current learning rate to schedule() in LearningRateScheduler (keras-team#8865)
  Simplify with from six.moves import input (keras-team#9216)
  fixed RemoteMonitor: Json to handle np.float32 and np.int32 types (keras-team#9261)
  Update tweet length from 140 to 280 in docs
  Add `depthconv_conv2d` tests (keras-team#9225)
  Remove `force` option in progbar
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.