sentry icon indicating copy to clipboard operation
sentry copied to clipboard

fix(files): close mmap in abstract file _prefetch

Open JoshFerge opened this issue 1 year ago • 3 comments

we seem to have a lot of warnings about this function and unclosed sockets. reading python docs, it seems like we should close this mmap. I'm not quite sure if this is the source of the log.

https://cloudlogging.app.goo.gl/Yu321395kTNxZFiWA

JoshFerge avatar May 10 '24 23:05 JoshFerge

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.37%. Comparing base (e0820a9) to head (24739ee). Report is 1110 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #70707       +/-   ##
===========================================
+ Coverage   50.46%   80.37%   +29.91%     
===========================================
  Files        6469     6517       +48     
  Lines      289328   298593     +9265     
  Branches    49888    52539     +2651     
===========================================
+ Hits       146002   239994    +93992     
+ Misses     142890    57865    -85025     
- Partials      436      734      +298     
Files Coverage Δ
src/sentry/models/files/abstractfile.py 93.97% <100.00%> (+68.57%) :arrow_up:

... and 2132 files with indirect coverage changes

codecov[bot] avatar May 11 '24 06:05 codecov[bot]

looking at this some more, the sockets are network based, not file based, so i don't think this PR is correct. looking further

JoshFerge avatar May 13 '24 17:05 JoshFerge

gonna guess its coming from https://github.com/getsentry/getsentry/blob/056e9cc337b98984b37ffd3b40a79adea07c32e5/getsentry/filestore.py#L141

JoshFerge avatar May 13 '24 17:05 JoshFerge

https://github.com/getsentry/getsentry/pull/13948

JoshFerge avatar May 14 '24 20:05 JoshFerge

this should probably happen anyway -- right now that file descriptor is torn down at an undefined time

asottile-sentry avatar May 15 '24 08:05 asottile-sentry

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

getsantry[bot] avatar Jun 06 '24 07:06 getsantry[bot]