python / cpython

The Python programming language
https://www.python.org/
Other
59.96k stars 29.02k forks source link

Free extension DLLs' handles during the Py_Finalize() #33239

Closed 95503518-be73-46db-a7a0-6593fec1d54b closed 5 years ago

95503518-be73-46db-a7a0-6593fec1d54b commented 23 years ago
BPO 401713
Nosy @gvanrossum, @tim-one, @loewis, @mhammond, @freddrake, @ericsnowcurrently
Files
  • None: None
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/tim-one' closed_at = created_at = labels = ['OS-windows'] title = "Free extension DLLs' handles during the Py_Finalize()" updated_at = user = 'https://bugs.python.org/markovitch' ``` bugs.python.org fields: ```python activity = actor = 'eric.snow' assignee = 'tim.peters' closed = True closed_date = None closer = None components = ['Windows'] creation = creator = 'markovitch' dependencies = [] files = ['2847'] hgrepos = [] issue_num = 401713 keywords = ['patch'] message_count = 9.0 messages = ['34518', '34519', '34520', '34521', '34522', '34523', '34524', '34525', '34526'] nosy_count = 7.0 nosy_names = ['gvanrossum', 'tim.peters', 'loewis', 'mhammond', 'fdrake', 'markovitch', 'eric.snow'] pr_nums = [] priority = 'normal' resolution = 'rejected' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue401713' versions = [] ```

    95503518-be73-46db-a7a0-6593fec1d54b commented 23 years ago
    freddrake commented 23 years ago

    Assigned to one of our Windows guys for review.

    freddrake commented 23 years ago

    Assigned to one of our Windows guys for review.

    mhammond commented 23 years ago

    I agree we should close handles that we can't use as extension modules.

    I am quite skeptical of the unloading of modules, tho. Python simply doesn't provide enough cleanup semantics to guarantee we are finished with the module at Py_Finalize() time. Indeed, extension modules are one main reason why Python often can not handle multiple Py_Initialize()/Py_Finalize() calls in the same process.

    I think that Python needs to grow module termination semantics. Something like, at Py_Finalize time:

    Try and find function "term_{module}" If function exists: call function free handle else: pass

    Thus - only modules that have gone to the trouble of providing a finalize function can be trusted to be unloaded.

    On one hand, the addition of the map means we _are_ in a better position for better finalization semantics on Windows. On the larger hand, module finalization semantics must be cross-platform anyway.

    So - while I acknowledge the problem, I don't believe this alone is a reasonable solution.

    Marking as postponed, and assigning back to Tim, so he can rule on the next step.... This came up a number of years ago, and Guido agreed "better" semantics were needed. Sounds like PEP material. I guess I _do_ care enough about this issue to own a PEP on it, as long as no-one needs the PEP finalized this year ;-)

    tim-one commented 23 years ago

    Mark, you got anything to say about this? Can't say I've ever noticed a problem here. Note that "the patch" is actually a .zip archive, and it takes a little effort to sort out what's what.

    95503518-be73-46db-a7a0-6593fec1d54b commented 23 years ago

    This patch is intended to fix the following problem: Python on Windows never frees DLLs loaded as extension. Whenever it's not a big problem when the interpreter is being used in a standart way, it becomes THE problem (or even a disaster) when the interpreter DLL is dynamically initialized/finalized from one process many times during single run. Moreover, even in case of single initialization there is a trap - DLLs loaded by mistake are unloaded only then a process finishes (e.g. suppose there is a foo.dll in the current directory and foo.dll is NOT a Python extension; "import foo" ends up with error, but foo.dll will be anging in process' address space!)

    This patch 1) frees a DLL handle in case of it has no proper initialization funcion 2) registers in an internal array all handles of successfully loaded dynamic extensions 2) frees all registered handles during Py_Finalize()

    Yakov Markovitch, markovitch@iso.ru

    95503518-be73-46db-a7a0-6593fec1d54b commented 23 years ago

    Yes, I agree with Mark, but there is the other side of the problem. Let's suppose that we have an application that uses the interpreter through dynamic loading (I mean through the LoadLibrary). It isn't likely to be directly, but the application can load/unload some other DLL which, in turn, uses an embedded interpreter. Now after freeing this DLL the application has ALL extensions which was used by this DLL loaded! (Though it hasn't the interpreter embedded at all!)

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 22 years ago

    Logged In: YES user_id=21627

    I recommend to reject this patch. If such a feature is implemented, it should be implemented uniformly across platforms - i.e. on Unix, appropriate dlclose calls should be issued.

    Furthermore, I don't see the problem with the DLLs being loaded. AFAIK, each DLL will be loaded only once, so even if the interpreter is stopped and started again, you get only one copy of the DLLs state per process, right? So what is the problem?

    Finally, it seems reasonable that people embedding the interpreter might need to customize its code. It is possible that the finalization procedure of user A won't work for user B, e.g. because they require state to survive different activations and deactivations.

    gvanrossum commented 22 years ago

    Logged In: YES user_id=6380

    Rejected, trusting MvL's judgement.