-
Notifications
You must be signed in to change notification settings - Fork 123
Added Finalizer to MemoryMappedFile #119
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
Conversation
Memory mapped files were not getting closed. TcImports attaches a dispose function to it's list of disposables to close the map, however, TcImports is never disposed in the "main" fsc.fs functions. So either TcImports needs to be disposed, a finalizer needs to be added to TcImports or a Finalizer needs to be added to MemoryMappedFile. Here the finalizer was added as close to the resources as possible.
Code to demonstrate the leak. If compiled as 32-bit it will quickly run out of address space and throw an out of memory exception. open Microsoft.FSharp.Compiler.SimpleSourceCodeServices
open System.IO
let run() =
let scs = SimpleSourceCodeServices()
let fn = Path.GetTempFileName()
let fn2 = Path.ChangeExtension(fn, ".fs")
let fn3 = Path.ChangeExtension(fn, ".dll")
File.WriteAllText(fn2, """
module M
type C() =
member x.P = 1
let x = 3 + 4
""")
let errors1, exitCode1 = scs.Compile([| "fsc.exe"; "-o"; fn3; "-a"; fn2 |])
()
for i = 1 to 200 do
run() |
@kevmal We tried to reproduce this on Windows but it didn't run out of memory using the example above. What operating system were you using? |
(Regardless we will switch away from using memory mapped files for the SimpleSourceCodeServices compilation methods, they just read in-memory) |
@quasilord Windows 8, Windows Server 2012 and I just ran it on Windows 7 64-bit targeting .NET 4.5 32-bit using nuget to get FSharp.Compiler.Service. It's important it's compiled to 32-bit since it actually just seems to run out of address space to map files to. Watching the "Working Set (Memory)" in task manager it throws the exception when the working set hits about 500MB, which on this machine takes roughly 60 seconds. When not targeting 32-bit I've seen the working set memory just continue 11GB+ with no exception thrown. |
Thank you, I was looking at "Memory (Private working set)" |
FCS 0.0.45 now opens binaries in-memory instead of using memory mapped files. Thanks for raising this issue, closing this since the visible issue has been addressed. Note this is still a good PR for basic code quality improvement but it would really belong in the core F# implementation from which this repo is derived (see http://github.com/fsharp/fsharp and http://visualfsharp.codeplex.com). So we shouldn't accept it here unless it is fixing an active FCS bug. |
fixes fsharp#119 closes fsharp#197 commit d4dd8389db1e97fe842529257da90a5218b7f195 Author: Vladimir Matveev <vladima@microsoft.com> Date: Tue Feb 3 16:20:52 2015 -0800 added test for settable extension property commit 5edc7ccacd90b8bbaba28ecdb7f9e5ecb1f3b171 Author: Vladimir Matveev <vladima@microsoft.com> Date: Tue Feb 3 16:20:26 2015 -0800 addressed CR feedback commit f17c57d250691a37811bdb82b7294b65ddc93676 Author: v2m <desco.by@gmail.com> Date: Sun Feb 1 14:55:02 2015 -0800 added tests for property completion in object creation expressions commit 38a073426d67da4eb33a8baa070ca8ba9420ea60 Author: v2m <desco.by@gmail.com> Date: Wed Jan 28 11:25:09 2015 -0800 initial revision of completions for properties in 'new' expressions
Memory mapped files were not getting closed. TcImports attaches a dispose function to it's list of disposables to close the map, however, TcImports is never disposed in the "main" fsc.fs functions. So either TcImports needs to be disposed, a Finalizer needs to be added to TcImports or a Finalizer needs to be added to MemoryMappedFile. Here the Finalizer was added as close to the resources as possible.