elixir-temp icon indicating copy to clipboard operation
elixir-temp copied to clipboard

Is the automatic cleanup after the process exits really working?

Open valo opened this issue 8 years ago • 9 comments

The README says:

By default, you have to cleanup the files by yourself, however, you can tell Temp to track the temporary files. You just need to call Temp.track (or the bang version Temp.track!) and you are done. Temporary files will be cleaned up automatically when the process exits

From my testing this doesn't seem to be the case and the files are still around after the process exits. Looking in the code I also don't see any place, where the current process will be monitored and the files cleaned up when it exits.

valo avatar Dec 10 '17 16:12 valo

Thanks for reporting.

This currently uses trap_exit with the terminate callback of GenServer so if the process is killed abruptly it will fail to clean. The following should however work without issue:

spawn fn ->
  Temp.track!
  IO.puts(Temp.mkdir!)
end

As this is more or less what happens when running tests, my primary use case for track, this issue has not bothered me much, but it should indeed be fixed. The correct approach would definitely to have an external process monitoring all the processes using Temp.track!, but I do not really have time right now, so help would be very welcome.

Thank you.

danhper avatar Dec 10 '17 16:12 danhper

This is very interesting. I didn't know about the terminate callback. The thing is that this temp file is definitely not removed https://github.com/santiment/sanbase2/blob/master/lib/sanbase_workers/import_github_activity.ex#L35 although the track! method is called at the beginning of the job and the job is performed in a separate process. I need to debug this further.

I am running this on kubernetes and I constantly get Disk Space errors after it runs for a while and also looking into the /tmp folder I see a lot of leftovers from old jobs...

valo avatar Dec 10 '17 21:12 valo

Am also trying Temp.track!() from an ExUnit test and can verify that the temporary file isn't being removed on exit.

Then whenever I try to call Temp.cleanup() manually, I get an temp tracker not started error.

aisrael avatar Nov 04 '19 03:11 aisrael

I guess since I'm using Temp.path!() and that doesn't actually register the generated path to the tracker: https://github.com/danhper/elixir-temp/blob/master/lib/temp.ex#L77

aisrael avatar Nov 04 '19 03:11 aisrael

Then whenever I try to call Temp.cleanup() manually, I get an temp tracker not started error.

I suppose it is because you are not calling Temp.cleanup() in the same process as Temp.track!(). This should be fixed so that there is a single process controlling the tracker but it is unfortunately not the case at the moment. PRs are very welcome.

danhper avatar Nov 04 '19 11:11 danhper

It's strange because even if I get the PID returned by Temp.track!() and pass that on to Temp.cleanup() I still get the temp tracker not started error.

When I have some time I'll try to look into it further. From what I've seen with other Elixir libraries, I think the current convention is to leverage Elixir 1.4 and have it auto-start your application (or, pass in runtime: false if you want to manually start_link everything). Then, in the Tracker GenServer maybe just use the module name as the GenServer process name so it's easier to find and cleanup afterwards?

I'll try to prototype something up for discussion.

aisrael avatar Nov 04 '19 13:11 aisrael

Weird. I can't seem to reproduce the problem anymore. Oh well, will just ping back when I have an MRE.

aisrael avatar Nov 05 '19 01:11 aisrael

As this is more or less what happens when running tests, my primary use case for track, this issue has not bothered me much, but it should indeed be fixed. The correct approach would definitely to have an external process monitoring all the processes using Temp.track!, but I do not really have time right now, so help would be very welcome.

I implemented this, before realizing that the current version of temp passes the same tests (and still has leftover directories if cleanup isn't called in the test case). I'm unsure if I did it right, though - https://github.com/konstantin-aa/elixir-temp/tree/external-process

From ExUnit docs for ExUnit.start(): "Starts ExUnit and automatically runs tests right before the VM terminates." Maybe there's no chance to clean up before the VM terminates?

konst-aa avatar Jun 13 '22 02:06 konst-aa

FYI, I encountered the same problem. After some searching, I found a new lib seems using a separate process to do all the tracking: https://github.com/CargoSense/briefly, which could potentially solve the problem.

LiboShen avatar Apr 04 '24 10:04 LiboShen