Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

threading.Thread: use target name if the name parameter is omitted #85999

Closed
vstinner opened this issue Sep 22, 2020 · 11 comments
Closed

threading.Thread: use target name if the name parameter is omitted #85999

vstinner opened this issue Sep 22, 2020 · 11 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 41833
Nosy @vstinner, @serhiy-storchaka, @ammaraskar
PRs
  • bpo-41833: threading.Thread now uses the target name #22357
  • 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:

    assignee = None
    closed_at = <Date 2020-09-23.21:23:21.682>
    created_at = <Date 2020-09-22.11:47:35.547>
    labels = ['library', '3.10']
    title = 'threading.Thread: use target name if the name parameter is omitted'
    updated_at = <Date 2020-10-12.08:58:07.448>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-10-12.08:58:07.448>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-09-23.21:23:21.682>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2020-09-22.11:47:35.547>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41833
    keywords = ['patch']
    message_count = 11.0
    messages = ['377314', '377320', '377328', '377330', '377333', '377405', '377407', '377411', '377421', '377422', '378479']
    nosy_count = 3.0
    nosy_names = ['vstinner', 'serhiy.storchaka', 'ammar2']
    pr_nums = ['22357']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue41833'
    versions = ['Python 3.10']

    @vstinner
    Copy link
    Member Author

    When debugging race conditions in a multithreaded application with many threads, it's hard to debug when threads have generic names like "Thread-3".

    I propose to use the target name in threading.Thread constructor if the name parameter is omitted.

    See for example bpo-41739 with "Dangling thread: <Thread(Thread-3, started daemon 4396088817936)>": the "Thread-3" name is not helpful. I suspect that the bug comes from threading.Thread(target=remove_loop, args=(fn, del_count)): with my proposed change, the thread would be called "target=remove_loop", which is more helpful than "Thread".

    Fall back on _newname() as usual if target.__name__ attribute does not exist.

    Attached PR implements this idea.

    @vstinner vstinner added 3.10 only security fixes stdlib Python modules in the Lib dir labels Sep 22, 2020
    @serhiy-storchaka
    Copy link
    Member

    And what if run different threads with the same target or with different targets with the same name? Would not "Thread-3" be more useful in this case?

    for i in range(10):
        Thread(target=print, args=(i,)).start()
    
    for i in range(10):
        def doit(i=i):
            print(i)
        Thread(target=doit).start()

    @vstinner
    Copy link
    Member Author

    And what if run different threads with the same target or with different targets with the same name?

    If multiple Thread objects using the same target (or different targets with the same name), they all get the same name using my PR 22357.

    Would not "Thread-3" be more useful in this case?

    Well, that's an open question. In my experience, "Thread" name is pretty useless.

    What if I modify my PR to add "-{counter}" (ex: "func-3" for target.__name__="func") to the default name? Reuse _counter().

    Pseudo-code:

            self._name = str(name)
            try:
                base_name = target.__name__
            except AttributeError:
                base_name = "Thread"
            if not self._name:
                self._name = "{base_name)-{_counter()}"

    @vstinner
    Copy link
    Member Author

    See also bpo-15500 "Python should support exporting thread names to the OS".

    @ammaraskar
    Copy link
    Member

    Having the target in the thread name definitely seems like a good idea and would help ease debugging, not just for our tests but when printing thread objects in general.

    I would agree with the suggestion of placing the counter in there for when multiple threads get spawned with the same target or maybe even using names like:

    Thread-1 (doit)
    Thread-2
    Thread-3 (print)

    might be better just for people who are familiar with them.

    @vstinner
    Copy link
    Member Author

    I rewrote my PR to generate names like "Thread-3 (func)", rather than just "func". So if two target functions have the same name, or if two threads use the same target, it remains possible to distinguish the two threads.

    @serhiy-storchaka
    Copy link
    Member

    Ideally, I would prefer separate counters for different names, and omitting a number for the first thread with unique name:

    doit
    doit-2
    Thread-1
    Thread-2
    print
    print-2
    doit-3

    But it is not feasible. It would require supporting a cache which can grow with every new thread and non-trivial code to avoid race conditions. So I am fine with Ammar's idea.

    @vstinner
    Copy link
    Member Author

    Ideally, I would prefer separate counters for different names

    IMO if you want to go at the level of details, I suggest you to generate yourself thread names:

    threads = [threading.Thread(name=f"MyThread-{i}") for i in range(1, 6)]

    Maintaining a list of thread names sounds overkill to me. It would be quite complicated and would increase the memory footprint, for little benefit.

    @vstinner
    Copy link
    Member Author

    New changeset 98c16c9 by Victor Stinner in branch 'master':
    bpo-41833: threading.Thread now uses the target name (GH-22357)
    98c16c9

    @vstinner
    Copy link
    Member Author

    I merged my change with "Thread-1 (func)" format: add the target name, but keep "Thread-3" to distinguish two different threads with the same target name.

    @vstinner
    Copy link
    Member Author

    Cool, my threading change works as expected! The name of the target function was logged in bpo-41739 issue.

    https://buildbot.python.org/all/#/builders/509/builds/172

    0:00:28 load avg: 3.58 [ 36/424/1] test_logging failed (env changed)
    Warning -- threading_cleanup() failed to cleanup 2 threads (count: 2, dangling: 3)
    Warning -- Dangling thread: <Thread(Thread-4 (removeTarget), started daemon 4396862667024)>
    Warning -- Dangling thread: <Thread(Thread-11 (removeTarget), started daemon 4396595259664)>
    Warning -- Dangling thread: <_MainThread(MainThread, started 4396920310576)>

    See "<Thread(Thread-4 (removeTarget), started daemon 4396862667024)>": the two leaked threads are running the removeTarget() function which come from MemoryHandlerTest.test_race_between_set_target_and_flush().

    By the way, Serhiy was right about the name of having an unique repr() even if the target is the same, since it's the case here ;-) Well "4396862667024" is different than "4396595259664", but it's more convinient to have short numbers like "Thread-4" and "Thread-11".

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants