Fix a resource leak when open a lfs file

[lunny]

No description provided.

[codecov-commenter]

Codecov Report

Merging #22043 (ee84390) into main (77f5035) will increase coverage by 48.15%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           main   #22043       +/-   ##
=========================================
+ Coverage      0   48.15%   +48.15%     
=========================================
  Files         0     1037     +1037     
  Lines         0   141409   +141409     
=========================================
+ Hits          0    68100    +68100     
- Misses        0    65173    +65173     
- Partials      0     8136     +8136     

Impacted Files	Coverage Δ
routers/web/repo/view.go	43.53% <0.00%> (ø)
routers/api/v1/repo/issue_tracked_time.go	38.66% <0.00%> (ø)
models/asymkey/ssh_key_deploy.go	55.24% <0.00%> (ø)
routers/api/v1/repo/subscriber.go	0.00% <0.00%> (ø)
models/organization/team_unit.go	21.05% <0.00%> (ø)
modules/git/remote.go	71.42% <0.00%> (ø)
services/webhook/matrix.go	73.38% <0.00%> (ø)
routers/web/goget.go	73.01% <0.00%> (ø)
modules/git/repo_tag_nogogit.go	46.66% <0.00%> (ø)
modules/git/blob_nogogit.go	62.02% <0.00%> (ø)
... and 1028 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[silverwind]

There is defer dataRc.Close() on line 255, shouldn't it take effect when the function returns?

Also, we probably shouldn't shadow the dataRc variable here.

[wxiaoguang]

There is defer dataRc.Close() on line 255, shouldn't it take effect when the function returns?

Also, we probably shouldn't shadow the dataRc variable here.

Yup, and even more, line 410

[lunny]

There is defer dataRc.Close() on line 255, shouldn't it take effect when the function returns?

Also, we probably shouldn't shadow the dataRc variable here.

This will close the previous dataRc because the next lines will override the dataRc variable.
I also don't like the shadown, so I created #22042 to resolve them. But for backport, this is also necessary.

[KN4CK3R]

Is the double defer dataRc.Close() correct? The docs say

The behavior of Close after the first call is undefined.

[silverwind]

This will close the previous dataRc because the next lines will override the dataRc variable.

But the previous Close should still execute on the first instance, right? When does go resolve the variable? At time of parsing or at time when the deferexecutes?

[lunny]

Looks like this PR is unnecessary. see https://go.dev/play/p/2d8BuU5ZVeg

[silverwind]

I'm actually quiet surprised that the code executed during defer sees the variable before the shadowing. I would have thought because defer executes last, it should see the last instance of the variable.