GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — FIX memoryview leaks and retrofit memory-manager as context-managers by ankostis · Pull Request #33 · gitpython-developers/smmap

This repository was archived by the owner on Jul 26, 2026. It is now read-only.

Conversation

@ankostis

@ankostis ankostis commented Oct 27, 2016

Copy link
Copy Markdown
Contributor

BREAKING API:

  • retrofit git.util.mman as context-manager,
    to release memory-mapped regions held.

    The mmap-manager(s) are re-entrant, but not thread-safe context-manager(s),
    to be used within a with ...: block, ensuring any left-overs cursors are cleaned up.
    If not entered, :meth:StaticWindowMapManager.make_cursor() and/or
    :meth:WindowCursor.use_region() will scream.

    Get them from smmap.managed_mmaps().

  • Simplify :class:SlidingWindowMapBuffer as create/close context-manager
    (no begin_access(), or end_access()).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
ankostis added 19 commits October 23, 2016 17:09
@ankostis
feat(enc): test with unicode tmpdir
2347f3d
Borrowed from gitpython-developers/GitPython#543
@ankostis
fix(leaks): attempt to plug the leaks & filter dead regions
883217b
+ Possible fixing gitpython-developers#31 by stop decrement on
destruction, rely only on `with...` resources.
+ feat(mmap): utility to check if regions have been closed (PY3-only).
@ankostis
chore(ver): bump 2.0.1-->2.1.0.dev0
43c5f23
@ankostis
fix(regs): fix/rename scream_if_closed()-->collect_closed_regions()
f10196f
@ankostis
chore(ver): bump 2.0.1-->2.1.0.dev1
88e2769
@ankostis
style(listuple): pep8, literals for empty lists/tuples
133dd1c
@ankostis
refact(region): rename offset _b --> _ofs
7c1eac7
@ankostis
refact(minor): use region.priv-func, close fd on same condition
bba086a
@ankostis
feat(mman): BREAKING API mman as context-manager to release regions
a2bc2d2
+ Add PY3 compat utilities
+ doc(changes, tutorial): update on mman usage
@ankostis
chore(ver): bump 2.1.0.dev1-->2.1.0.dev3
01df7f3
@ankostis
feat(mman-contxt): opt-out not to scream if mman not entered
bf68f77
@ankostis
fix(leaks): attempt to gc-collect before region-collect
4598966
futile...
@ankostis
fix(mman): exit log-msg were missing left-overs arg, log as debug
d0bd74e
@ankostis
refact(buf): also use SlidingWindowMapBuffer as optional context-manager
e33235a
+ doc(tutorial): update use-cases
+ doc(changes): new bullet.
@ankostis
refact(TCs): unittestize assertions
33f12e6
@ankostis
refact(buf): simplify API - no begin/end after construct
8489c31
+ feat(mman): report missed exits.
@ankostis
style(mman): move managed_mmaps() closer to 2 mmans
d81dc1d
@ankostis
fix(leaks): FIX memoryview leak in Windows
9ba1649
+ All gitdb TCs now pass without explit release!
@ankostis
chore(ver): bump 2.1.0.dev3-->2.1.0.dev4
144891b
@ankostis ankostis added the tag.leaks label Oct 27, 2016
@coveralls

coveralls commented Oct 27, 2016

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.7%) to 93.116% when pulling 144891b on ankostis:leaks2 into 6e55a1c on gitpython-developers:master.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment
@coveralls

coveralls commented Oct 27, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.7%) to 93.116% when pulling 144891b on ankostis:leaks2 into 6e55a1c on gitpython-developers:master.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ankostis

ankostis commented Oct 27, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@Byron can you check these lines where a memoryview is crated?

It took me the half month to realize that this is leaking badly, and I want you to check the history of this commits leading up to it, because I cannot understand whether they were mistake or intentional, and what would be the repercussions (ie, performance?).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Byron Byron self-assigned this Dec 8, 2016
@Byron

Byron commented Dec 8, 2016

Copy link
Copy Markdown
Member

@ankostis Sorry for the very late reply. I am no holiday now, and am working through everything that piled up.
About the matter: I don't understand why this would leak in the first place. Does that mean it's a python 3 only issue? Also I wonder how anything can leak in the first place unless there are dependency cycles.
The comment in the respective are of the code also contains an alternative. It appears that it was chosen just for performance reasons. Maybe alternatives do by now exist too.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ankostis

ankostis commented Dec 8, 2016

Copy link
Copy Markdown
Contributor Author

Yes, it is a python-3 issue, because the destructors are not deterministic.
Untill all memoryview for some mmemp have close, neither the memmap nor its file can be collected.

Now, in python3, you can slice memmaps as if they were memoryview, so they are not strictly needed, unless it is a performance issue. From some quick experiments I had done, I could not see any benefits, quite the contrary, but it's been a long time and I don't remember exactly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Byron

Byron commented Dec 8, 2016

Copy link
Copy Markdown
Member

If using direct access would be a solution, then it probably should be used instead. Correctness over performance I would say.
The main driver behind using the 'buffer' interface was to prevent copying of the slices that are read, and thus maintain the awesome properties of a memory map even in python. I do remember me digging around in python source code to see if copies were made.
However, now is a different time, and slicing memory maps directly in python 3 sounds appealing to me.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@EliahKagan EliahKagan mentioned this pull request Dec 13, 2023
SymbolicReference.set_reference may not roll back properly on error gitpython-developers/GitPython#1669
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@Byron Byron

Labels

tag.leaks

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@ankostis @coveralls @Byron
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.