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

Multi threaded use of RCON #13

Open
gilesknap opened this issue Aug 30, 2022 · 8 comments
Open

Multi threaded use of RCON #13

gilesknap opened this issue Aug 30, 2022 · 8 comments

Comments

@gilesknap
Copy link

gilesknap commented Aug 30, 2022

Should RCON be thread safe if you open a new client per thread?

I have been successfully using this but have started to see a few instances of where the response that should go back to thread A is concatenated to the response for thread B, Thread A gets a null response when this happens.

A minimal demo of the issue is here:

import threading
from time import sleep

from mcipc.rcon.je import Client

from mciwb.nbt import parse_nbt


def get_player_pos(name: str, wait: float):
    client = Client("nuc2", port=30555, passwd="spider")
    client.connect(True)
    for i in range(100):
        x = client.data.get(entity="@p", path="Pos")
        print(name, parse_nbt(x))
        sleep(wait)

def go():
    t1 = threading.Thread(target=get_player_pos, args=("t1", 0.2))
    t2 = threading.Thread(target=get_player_pos, args=("t2", 0.3))
    t1.start()
    t2.start()

The results of this test show a clash on the first call to both threads. If I adjust the wait to be the same I see more clashes.

In [1]: go()

Error parsing NBT text: TransformerScorn has the following entity data: [430.74282982933767d, 178.11815687065814d, -1507.2116496515248d]TransformerScorn has the following entity data: [430.74282982933767d, 178.11815687065814d, -1507.2116496515248d] 

 ERROR: Extra data: line 1 column 62 (char 61)
t2 None
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t2 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
Error parsing NBT text: TransformerScorn has the following entity data: [430.74282982933767d, 178.11815687065814d, -1507.2116496515248d]TransformerScorn has the following entity data: [430.74282982933767d, 178.11815687065814d, -1507.2116496515248d] 

 ERROR: Extra data: line 1 column 62 (char 61)
t2 None
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t2 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t1 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
t2 [430.74282982933767, 178.11815687065814, -1507.2116496515248]
@gilesknap
Copy link
Author

#14 has fixed this issue. The library can now do crazy stuff like this, tracking every currently airborne snowball in a separate thread :-)

https://github.com/gilesknap/mciwb/blob/bfbb339ad44666b841478b517d61c1159db325fd/src/demo/snowball.py#L1

@gilesknap
Copy link
Author

@conqp do you get why the code above has an issue? I've looked in Client and it creates its own socket. Thus if I have two Client objects they are using separate sockets and there should be no crossover.

Unless Minecraft's RCON server itself is at fault?

@gilesknap
Copy link
Author

I can confirm that adding global lock to mcipc Client fixes the above. BUT if I run two separate processes together then I get the same issue back again.

I think this implies the issue does lie at the server?!!??

@BreathXV
Copy link

Was just about to inquire about this as my application uses multi threading to handle each client.

@gilesknap
Copy link
Author

gilesknap commented May 25, 2024

@BreathXV are you seeing any issues with multi threading? Did you need to do anything to mitigate the problem I'm seeing?

@conqp
Copy link
Owner

conqp commented May 25, 2024

If each thread has its own client this should™ not be an issue.
In any case, I'm currently on vacation for the next two weeks, so I cannot work on this before 2024-06-03.

@BreathXV
Copy link

@gilesknap No, I haven't fully tested my application yet but I've been running each instance of the client as a thread, then using it to execute commands, then I start a new thread for another client.

I'll do some testing at some point today and let you know.

@gilesknap
Copy link
Author

I'll do some testing at some point today and let you know.

If you do see problems then I would expect the pr I referenced above would be a workaround (but implemented at the wrong level, really)

However my testing showed the same issue even with that fix if I ran two clients on separate workstations. This implies the rcon server itself has an issue but that is rather surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants