Add missing call to finalize the interpreter on NatSpeak shutdown

[drmfinlay]

This pull request adds a missing call to Py_Finalize() in COM\appsupp.cpp. This should allow things like the atexit module to work.

I have set this as a draft PR for the moment. Still need to compile and test that this works fine.

[drmfinlay]

Had some trouble setting Natlink up in order to test this, but I can confirm the change hasn't broken anything. The atexit module works properly now, as do other tasks normally performed prior to the termination of python.exe. This PR is ready to be merged.

I did notice an interesting caveat in the Python/C API Reference Manual that applies to Natlink:

Notice that Py_Finalize() does not free all memory allocated by the Python interpreter, e.g. memory allocated by extension modules currently cannot be released.

Embedding Python (final paragraph)
https://docs.python.org/2/c-api/intro.html#embedding-python

[LexiconCode]

Had some trouble setting Natlink up in order to test this, but I can confirm the change hasn't broken anything. The atexit module works properly now, as do other tasks normally performed prior to the termination of python.exe. This PR is ready to be merged.

I did notice an interesting caveat in the Python/C API Reference Manual that applies to Natlink:

Notice that Py_Finalize() does not free all memory allocated by the Python interpreter, e.g. memory allocated by extension modules currently cannot be released.

Embedding Python (final paragraph) docs.python.org/2/c-api/intro.html#embedding-python

I did mention this to Doug on telegram, he thought it wasn't too much of a concern as it's not reinitialized too often. For reference

https://stackoverflow.com/questions/42971734/memory-leak-when-embedding-python-into-my-application

Could PyGC_Collect() be called right before Py_Finalize()?

[LexiconCode]

I do not think this is do to the pr.

Just a heads up, I think there's a bug when NatLink shuts down.

So I loaded it with an empty _grammar.py. loads correctly. First load. Shut down dragon through the tray. Reload dragon after the tray icon disappears:

debug says triggering load/reload process
skipping unchanged loaded module: _grammar.

Process from 'messaging from natlink' is still running. Despite dragon being closed. Once closed from task manager when dragon restarts it functions as expected.

[drmfinlay]

I agree, not a big deal. I just thought it was interesting.

…

On Fri, 02 Sep 2022 08:04:59 -0700 LexiconCode ***@***.***> wrote: > Had some trouble setting Natlink up in order to test this, but I can confirm the change hasn't broken anything. The `atexit` module works properly now, as do other tasks normally performed prior to the termination of python.exe. This PR is ready to be merged. > > I did notice an interesting caveat in the _Python/C API Reference Manual_ that applies to Natlink: > > > Notice that Py_Finalize() does not free all memory allocated by the Python interpreter, e.g. memory allocated by extension modules currently cannot be released. > > _Embedding Python_ (final paragraph) [docs.python.org/2/c-api/intro.html#embedding-python](https://docs.python.org/2/c-api/intro.html#embedding-python) I did mention this to Doug on telegram, he thought it wasn't too much of a concern as it's not reinitialized too often. For reference https://stackoverflow.com/questions/42971734/memory-leak-when-embedding-python-into-my-application -- Reply to this email directly or view it on GitHub: #129 (comment) You are receiving this because you were assigned. Message ID: ***@***.***>

[drmfinlay]

Could PyGC_Collect() he called right before Py_Finalize()?

There is no need. Py_Finalize(), or, more directly, Py_FinalizeEx(), already calls this function. See pylifecycle.c line 1830.

[drmfinlay]

Yes, the behaviour you mention is unrelated. I didn't get this when I tested the change yesterday.

I have previously seen natspeak.exe hang around after it should have exited. Not sure there's anything that can be done about that. Maybe it's doing some maintenance.

[quintijn]

The issue #184 points to this issue, to not forget.