bug#15647: 24.3.50; python.el does not clean up temp file

bug#15647: 24.3.50; python.el does not clean up temp file

[Jorgen Schaefer]

python.el will use temporary files to communicate with the inferior
python process. These temporary files are never cleaned up.

To reproduce, simply run M-x run-python and check the temp dir. It has a
"py?????" file in it. Kill the buffer and repeat, such files will
accumulate.

The following simple patch will delete the file after loading it.


--- lisp/progmodes/python.el	2013-10-07 18:51:26 +0000
+++ lisp/progmodes/python.el	2013-10-18 18:47:02 +0000
@@ -2048,6 +2048,8 @@
                (file-name (or (buffer-file-name) temp-file-name)))
           (with-temp-file temp-file-name
             (insert string)
+            (insert (format "\n\nimport os ; os.remove('''%s''')\n"
+                            temp-file-name))
             (delete-trailing-whitespace))
           (python-shell-send-file file-name process temp-file-name))
       (comint-send-string process string)

bug#15647: 24.3.50; python.el does not clean up temp file

[Andreas Röhler]

Am 18.10.2013 20:51, schrieb Jorgen Schaefer:
> python.el will use temporary files to communicate with the inferior
> python process. These temporary files are never cleaned up.
>
> To reproduce, simply run M-x run-python and check the temp dir. It has a
> "py?????" file in it. Kill the buffer and repeat, such files will
> accumulate.
>
> The following simple patch will delete the file after loading it.
>
>
> --- lisp/progmodes/python.el	2013-10-07 18:51:26 +0000
> +++ lisp/progmodes/python.el	2013-10-18 18:47:02 +0000
> @@ -2048,6 +2048,8 @@
>                  (file-name (or (buffer-file-name) temp-file-name)))
>             (with-temp-file temp-file-name
>               (insert string)
> +            (insert (format "\n\nimport os ; os.remove('''%s''')\n"
> +                            temp-file-name))
>               (delete-trailing-whitespace))
>             (python-shell-send-file file-name process temp-file-name))
>         (comint-send-string process string)
>
>
>
>

Beside of other nasty side-effects to expect, when the code to send is extended that way:
Why relying on Python process in order to delete a file?

bug#15647: 24.3.50; python.el does not clean up temp file

[Stefan Monnier]

> Beside of other nasty side-effects to expect, when the code to send is
> extended that way: Why relying on Python process in order to delete
> a file?

IIUC the purpose is to make sure it's erased and to make sure it's
erased *after* the use.  Whether it does that, I don't know.
But doing it in Elisp would otherwise require detecting the next prompt
to figure out when the temp file can be erased.


        Stefan

bug#15647: 24.3.50; python.el does not clean up temp file

[Andreas Röhler]

Am 19.10.2013 23:51, schrieb Stefan Monnier:
>> Beside of other nasty side-effects to expect, when the code to send is
>> extended that way: Why relying on Python process in order to delete
>> a file?
>
> IIUC the purpose is to make sure it's erased and to make sure it's
> erased *after* the use.  Whether it does that, I don't know.
> But doing it in Elisp would otherwise require detecting the next prompt

Don't think so. Once the file is sent to process, it's sent.

A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.
OTOH python-mode will do an error-checking anyway, will point to it, so deleting might made depend from this.

For several reasons in favor of an Emacs Lisp solution as far as possible.
Executing code should not change the state of Python more than the code demands.
If a module isn't there, Emacs should it not provide it slightly - unless there is no other way to make things work.

If Emacs changes the Python state without the user is alerted, error tracking of Python itself might puzzle people.

Well, in the precise case it's probably rather a sanitary issue, a matter of code quality.

> to figure out when the temp file can be erased.
>
>
>          Stefan
>

bug#15647: 24.3.50; python.el does not clean up temp file

[Eli Zaretskii]

> Date: Sun, 20 Oct 2013 09:51:32 +0200
> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
> Cc: 15647@debbugs.gnu.org
> 
> > IIUC the purpose is to make sure it's erased and to make sure it's
> > erased *after* the use.  Whether it does that, I don't know.
> > But doing it in Elisp would otherwise require detecting the next prompt
> 
> Don't think so. Once the file is sent to process, it's sent.

That's Unix-speak: deleting a file that is potentially in use.
Outside of Posix, deleting a file immediately after submitting it to a
process will at best fail (because the process is still using it), and
at worst cause trouble (because the process didn't yet have enough
time to even open the file).

In fact, in this case, even on Unix this proposal will cause trouble,
because the command sent to Python might take time to execute on the
Python side, and we might already have deleted the file when Python
tries to open it.

I think in this case the better place to delete the file is in
python-shell-send-file, as part of the command sent to Python, because
that's where we know that the file was used up and closed by the
Python interpreter.

> A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
> Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.

No, it's an argument to add independent logging facilities to
python.el, IMO.  IOW, if python.el wants to have debugging features,
it should have them without relying on the Python interpreter and
without interfering with the "normal" workflow (whereby the file is
deleted after being used).  Relying on a temporary file to remain in
the filesystem for prolonged periods of time is not a good idea
anyway.

bug#15647: 24.3.50; python.el does not clean up temp file

[Andreas Röhler]

Am 20.10.2013 17:33, schrieb Eli Zaretskii:
>> Date: Sun, 20 Oct 2013 09:51:32 +0200
>> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
>> Cc: 15647@debbugs.gnu.org
>>
>>> IIUC the purpose is to make sure it's erased and to make sure it's
>>> erased *after* the use.  Whether it does that, I don't know.
>>> But doing it in Elisp would otherwise require detecting the next prompt
>>
>> Don't think so. Once the file is sent to process, it's sent.
>
> That's Unix-speak: deleting a file that is potentially in use.
> Outside of Posix, deleting a file immediately after submitting it to a
> process will at best fail (because the process is still using it), and
> at worst cause trouble (because the process didn't yet have enough
> time to even open the file).
>
> In fact, in this case, even on Unix this proposal will cause trouble,
> because the command sent to Python might take time to execute on the
> Python side, and we might already have deleted the file when Python
> tries to open it.
>
> I think in this case the better place to delete the file is in
> python-shell-send-file, as part of the command sent to Python, because
> that's where we know that the file was used up and closed by the
> Python interpreter.
>

Agreed.

>> A remaining question: what to do if the command fails? Maybe the temp file is of interest than?
>> Which might be an argument to do it from Python, as the error might prevent further action, i.e. deleting.
>
> No, it's an argument to add independent logging facilities to
> python.el, IMO.  IOW, if python.el wants to have debugging features,
> it should have them without relying on the Python interpreter and
> without interfering with the "normal" workflow (whereby the file is
> deleted after being used).  Relying on a temporary file to remain in
> the filesystem for prolonged periods of time is not a good idea
> anyway.
>

Agreed also. However don't think it's a big issue if the one or other file remains. Just it should not happen all the time.
BTW python-mode.el does this behind an unwind-protect, so errors should not go into the way.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Eli Zaretskii wrote:

> In fact, in this case, even on Unix this proposal will cause trouble,
> because the command sent to Python might take time to execute on the
> Python side, and we might already have deleted the file when Python
> tries to open it.

Whut?
It's python itself that deletes the file, as the last thing in the file.
So it's already done whatever it was supposed to actually do.
It's like a self-deleting shell script.
FWIW, the patch looks fine to me (I say this was zero testing of it).

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Glenn Morris wrote:

> FWIW, the patch looks fine to me (I say this was zero testing of it).
                                              s/was/with

bug#15647: 24.3.50; python.el does not clean up temp file

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Andreas Röhler <andreas.roehler@easy-emacs.de>,
>   15647@debbugs.gnu.org
> Date: Sun, 20 Oct 2013 14:27:46 -0400
> 
> Eli Zaretskii wrote:
> 
> > In fact, in this case, even on Unix this proposal will cause trouble,
> > because the command sent to Python might take time to execute on the
> > Python side, and we might already have deleted the file when Python
> > tries to open it.
> 
> Whut?
> It's python itself that deletes the file, as the last thing in the file.

I was talking about Andreas's proposal, not about the patch.  Sorry
for being unclear.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Eli Zaretskii wrote:

> I was talking about Andreas's proposal, not about the patch.  Sorry
> for being unclear.

Probably my bad for not reading properly.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Version: 24.4

Eli Zaretskii wrote:

> I think in this case the better place to delete the file is in
> python-shell-send-file, as part of the command sent to Python, because
> that's where we know that the file was used up and closed by the
> Python interpreter.

Agreed; done.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Thanks. It bugs me that every `make check' in Emacs trunk leaves several
python temp files behind, so I hope something like this gets applied.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Version: 24.4

Thanks; applied.

bug#15647: 24.3.50; python.el does not clean up temp file

[Glenn Morris]

Actually on second thought, un-applied and reopened.

bug#15647: 24.3.50; python.el does not clean up temp file

[Andreas Röhler]

Am 23.11.2013 05:21, schrieb Glenn Morris:
>
> Actually on second thought, un-applied and reopened.
>
>
>
>

Seems wise. Nonetheless, may you tell your reflections?