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

Fix python thread #1703

Merged
merged 1 commit into from
Dec 1, 2024
Merged

Fix python thread #1703

merged 1 commit into from
Dec 1, 2024

Conversation

michaelortmann
Copy link
Member

@michaelortmann michaelortmann commented Oct 17, 2024

Found by: Empus
Patch by: thommey and michaelortmann
Fixes:

One-line summary:
Fixes [20:20:20] !!! writing to nonexistent socket: 9 when using putlog() or other functions using sockets not available via thread local storage returned via threaddata() in python thread

Additional description (if needed):

Test cases demonstrating functionality (if applicable):
test.py:

import threading
import time
 
from eggdrop.tcl import putlog
 
def run_task():
    def bg_task():
       putlog("Inside background task bg_task()")
       time.sleep(5)
       putlog("Inside background task bg_task() 2")
 
    try:
        thread = threading.Thread(target=bg_task)
        putlog("Starting thread...")
        thread.start()
        putlog("Thread started successfully")
    except Exception as e:
        putlog(f"Thread failed to start: {e}")
 
putlog("Loaded Python thread test.")

Terminal 1:

$ ./eggdrop -nt BotA.conf
[...]
[20:39:30] tcl: builtin dcc call: *dcc:tcl testuser 9 pysource test.py
[20:39:30] tcl: evaluating .tcl pysource test.py
[20:39:30] Loaded Python thread test.
[20:39:30] tcl: evaluated .tcl pysource test.py, user 3.303ms sys 0.000ms
[20:39:38] tcl: builtin dcc call: *dcc:python testuser 9 run_task()
[20:39:38] Starting thread...
[20:39:38] Inside background task bg_task()
[20:39:38] Thread started successfully
[20:39:43] Inside background task bg_task() 2

Terminal 2:

$ telnet 127.0.0.1 3333
[...]
.tcl pysource test.py
[20:39:30] tcl: builtin dcc call: *dcc:tcl testuser 9 pysource test.py
[20:39:30] tcl: evaluating .tcl pysource test.py
[20:39:30] Loaded Python thread test.
[20:39:30] tcl: evaluated .tcl pysource test.py, user 3.303ms sys 0.000ms
Tcl: 
.python run_task()
[20:39:38] tcl: builtin dcc call: *dcc:python testuser 9 run_task()
[20:39:38] Starting thread...
[20:39:38] Inside background task bg_task()
[20:39:38] Thread started successfully
Python: None
[20:39:43] Inside background task bg_task() 2

@thommey
Copy link
Member

thommey commented Nov 23, 2024

LGTM, the underlying idea is that the dccsockets etc. of the main thread are separated from the one in Tcl threads because that is necessary.
From Python however, it should be (famous last words) safe to access the main thread's socket table, so this PR makes it globally accessible, with only Tcl threads having their own.
The safety requirement is that Python is only called when the main thread isn't operating on them and right now, Python is only called during Tcl binds and in select().

@vanosg vanosg added the Python label Dec 1, 2024
@vanosg vanosg added this to the v1.10.1 milestone Dec 1, 2024
@vanosg vanosg merged commit 14c181b into eggheads:develop Dec 1, 2024
24 checks passed
@michaelortmann michaelortmann deleted the python.thread branch December 2, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants