Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Refactorization of_cursesmodule.c to fix reference leaks #123961

Closed
Assignees
picnixz
Labels
extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement
@picnixz

Description

@picnixz

Feature or enhancement

gh-123962: global dictionary removal

In_cursesmodule.c, there is a global reference to the module's dictionary. The reason is that we cannot add all constants when the module is being initialized because some constants requirecurses.initscr() to be called beforehand.

Instead of storing the module's dictionary in a global variable, we can access it incurses.initscr() usingPyModule_GetDict.

gh-124047: global variable name cleanup

Some global variables are modified in_cursesmodule.c by other functions and their names can be confused with a local variable name. I wanted to use uppercase names but Victor argued that this should be reserved for C macros (#123910 (comment)) and suggested keeping prefixed lowercase names.

gh-124729: use a global state object to hold global variables

Once we are done with the above changes, we introduce a global module's state object. This state will hold the error and window type (which needs to be converted into a heap type) that are currently global variables.

gh-124907: improve C API cleanup

We improve the interface for creating and destroying the underlying C API and its associated capsule. Once we transform the Window type into a heap type, we'll also add the correspondingtraverse() andclear() callbacks to the capsule object.

gh-124934: transform window type into a heap type

In this task, we make curses window type a heap type instead of a static one. Since the module is still not yet a multi-phase initialization module, we cannot entirely clean-up all references.

gh-125000: make the modulefree-threaded friendly process-wide

The module should not be allowed to be exec'ed() more than once by process. This is achieved by adding a global flag.In addition, we should make the read/write operations on tbe global flags atomic so that free-threaded builds do not have race conditions.

gh-124965: implementPEP-489 forcurses

The last task takes care of changing the module into aPEP-489 compliant one. It will also take care of the long-standing reference leaks in curses.


Some extra and optional tasks at the end:

Error messages improvements

Some functions say that "X() returned ERR" but the function X is not always the correct one (e.g., in_curses_window_echochar_impl, it says thatechochar returned ERR but this depends on whether we usedechochar orwechochar). I think we need to really use the name of the function that was called at runtime (some functions do this but not all).

The idea is to extend the exception type so that it also has two attributes, namelycalled_funcname andcurses_funcname (subject to bikeshedding). Thecalled_funcname value would contain the incriminating function (i.e. where the exception was raised) whilecurses_funcname would contain the runtime C function that was used (which may differ in many cases from the actual curses Python function).

To that end, we need the exception type to be a heap type, since a static type would be 1) an overkill 2) still leaks (and we don't like leaks).

Cosmetic changes

I can't say I'm maintaining curses but I can say that it's tiring to work with a file of more 5k lines. Many lines are actually caused by the fact that we have both the implementation for the window object and for the module's method. But these two can probably be split into two files. I don't know if this is suitable but it would probably help maintining the code (both take roughly 2k lines of code). Here are the various cosmetic points I'd like to address:

  • Make the macros more readable (there are packed and honestly, for a first-time contributor, hard to parse).
  • Split the module into multiple.c files. This is not necessarily mandatory because we would probably need to export some functions that are shared or add some internal header so it might overcomplicate the task.
  • Simplify some code paths (just some if/else that could be cleaner or things like that).
  • Put whitespaces in function calls (honestly, when you seefunc(y,x, something), it's hard to read).

Those cosmetic changes could be helpful to newcomers, since no one is really interested in this module. But those are just things I had in mind while refactoring the module, none of them are really required (and I'm now quite used to).

EDIT (03/10/24)

I'll just give up on splitting the files. There are too many inter-dependencies between functions that it makes any PR for that too hard to review. In addition, we would end up exporting too many un-necessary symbols.

Related Work

Linked PRs

Metadata

Metadata

Assignees

Labels

extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions


    [8]ページ先頭

    ©2009-2025 Movatter.jp