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

Under Windows, file handles stay open even though the FSHandle instances are closed #38

Closed
j4yk opened this issue Apr 16, 2017 · 9 comments
Labels

Comments

@j4yk
Copy link
Collaborator

j4yk commented Apr 16, 2017

Inspecting the VM process with Process Explorer revealed that the file handles for pack files stay open, even though the corresponding FSHandle instances are already garbage collected or closed, as far as the object memory is concerned.

@fniephaus can you please check if the same is true for Mac OS? Just work a while in your image and check the process after some git operations.

Snippets to find (officially) open files:

FSStream allSubInstances select: [:ea | (ea instVarNamed: 'handle') reference filesystem = FileSystem disk]

FSFileHandle allInstances select: [:ea | ea isOpen]

The tools to inspect the actually open file descriptors or handles vary by OS, of course.

@j4yk j4yk added the bug label Apr 16, 2017
@fniephaus
Copy link
Member

I don't even get to see any instances, because of #33.
So currently, both statements return an empty collection in my image.

@fniephaus
Copy link
Member

Okay, so I can confirm that this is also the case on macOS. It is actually the root cause for #33, because on macOS Sierra the default limit for the number of open files per process appears to be 256. Raising the limit temporarily "fixes" #33 until the new limit is hit. After a few seconds, unfortunately, the number of open pack files already is larger than 10,000.
It'd be great if you could fix this next, as it makes Squot and even the entire image unusable...

@fniephaus
Copy link
Member

I can only guess at this point, but is it possible that the unpacker ends up in an endless recursion for some reason? unpackWithoutIndexFile also looks a little weird. Why decrement count when using timesRepeat:?
StandardFileStream has a finalize method, maybe FSFileHandle also needs one? (Tried this already, but nothing really changed).
It's very weird that this only happens with pack files...

@j4yk
Copy link
Collaborator Author

j4yk commented Apr 17, 2017

The image does not hanging there is no forking going on in FileSystem-Git so there should not be an endless recursion. The count temporary is doubly used as a "remaining" counter. Do not remember from the top of my head what it is used for (sitting in a car atm).

A missing close on garbage collection would explain why the OS handles remain open, but no open FSHandles exist. Don't know if I'll be able to find and fix the problem before my departure to Finland, though.

@fniephaus
Copy link
Member

Okay, so finalize seems reasonable, but does not solve the problem. It seems that each pack file is opened multiple times without closing it. This is what it looks like in macOS' Activity Monitor:

9
/Users/fniephaus/code/Squot/.git/objects/pack/pack-880f8345f2b0117652489bd69ec4d3f5cc28b533.pack
10
/Users/fniephaus/code/Squot/.git/objects/pack/pack-9e1de978700c6d1a7f468a1c95c0b952f44db11b.pack
11
/Users/fniephaus/code/Squot/.git/objects/pack/pack-880f8345f2b0117652489bd69ec4d3f5cc28b533.pack
12
/Users/fniephaus/code/Squot/.git/objects/pack/pack-880f8345f2b0117652489bd69ec4d3f5cc28b533.pack
13
/Users/fniephaus/code/Squot/.git/objects/pack/pack-9e1de978700c6d1a7f468a1c95c0b952f44db11b.pack
14
/Users/fniephaus/code/Squot/.git/objects/pack/pack-880f8345f2b0117652489bd69ec4d3f5cc28b533.pack
15
/Users/fniephaus/code/Squot/.git/objects/pack/pack-880f8345f2b0117652489bd69ec4d3f5cc28b533.pack
...

@j4yk
Copy link
Collaborator Author

j4yk commented Apr 17, 2017

I think I solved it in 721bc9d. Implementing finalize does not suffice, you have to add the object to a WeakRegistry to have it finalized. During my tests, all handles are properly closed now, please check.

@fniephaus
Copy link
Member

Thanks, I realized that I need a WeakRegistry, but added it to FSHandle, so that FSMemoryHandles are also closed when GCed. However, my version is still not working, but checking again with yours. Nonetheless, it looks like Squot/Squit does lots of (sometimes redundant) I/O operations. Not sure where caching would be useful...

@j4yk
Copy link
Collaborator Author

j4yk commented Apr 17, 2017

FileSystem-Git is the likely culprit, yes. Currently, each time an object is looked up, a new file stream is opened. There is some caching in place already, but it could be better or is not very helpful yet. The Squit browser could also do less when a reference has not changed, for example. Or generating the object list should not involve extracting the artifacts from the files, etc. There is a lot of work to do (or to save...), when there is time.

@fniephaus
Copy link
Member

Sounds good! 😃 And even better: your version seems to be working just fine. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants