munawarb / Python-Kill-Thread-Extension

This is a Python module written in C that lets a Python developer kill a thread.
34 stars 7 forks source link

OverflowError on Python2.7 #4

Closed georg90 closed 5 years ago

georg90 commented 5 years ago

This will give an OverflowError signed integer is greater than maximum in Python2.7

Any hints?

munawarb commented 5 years ago

Hmm, I'm wondering if we can change it to use some sort of Big Integer class. The problem is the thread ID is considered to be an opaque object as far as C is concerned, so it only has issues in that module. I'll take a look and see what I can come up with. Thanks for the heads-up! Have you tried it using Python 3? Do you get the same problem?

georg90 commented 5 years ago

Haven't tried Python 3 (the HW is only running on 2.7) - but then we would use the other code sample anyway, right?

We thought about using a float for the ident? I am really not into C at all..

munawarb commented 5 years ago

What hardware are you running it on? It could be an issue with word size. Did you build the module on the system in question or did you build on a different machine and then transfer it over?

georg90 commented 5 years ago

Running it on a Pepper robot from Softbanks robotics. Will try to compile on the robot itself. It was compiled on MacOS, so this could be an issue.

munawarb commented 5 years ago

Ok. Let me know how it goes. Since you compiled it on a Mac and are using it on a micro processor this could definitely be the issue because the size of an integer is probably smaller on your hardware than it is on the Mac. If this doesn't resolve it, I'll see what I can do. Some sample code would help as well.

georg90 commented 5 years ago

I tried compiling on the robot, but it fails to have the necessayr ressources like gcc. I think there is no easy way to install those anyway.

I compiled in a Linux VM using the same python version and got the same result: except signed integer is greater than maximum

This is the custom restart method I use:

    def restartThread(self):
        class CustomThread(threading.Thread):
            def __init__(self, function):
                self.running = False
                self.stopped = False
                self.function = function
                super(CustomThread, self).__init__()

            def is_alive(self):
                return not self.stopped

            def start(self):
                self.running = True
                super(CustomThread, self).start()

            def run(self):
                while self.running:
                    self.function()

            def end(self):
                if self.is_alive():
                    threader.killThread(self.ident)
                    self.running = False
                    self.stopped = True
munawarb commented 5 years ago

Ok. Can you give me some sample code that you ran on Linux?

georg90 commented 5 years ago

What kind of sample code?

I use the Thread implementation as above and then start my websocket connection

self.recognize_thread = CustomThread(function=self.recognize_using_websocket)
self.recognize_thread.start()

I then want to kill self.recognize_thread using threader. I am doing this by calling self.recognize_thread.end() but this returns the above error. The ident of the process is something like this, seems quite big..

ident: 123145496125440
georg90 commented 5 years ago

We kind off got around this issue (since we don't need to kill it anymore), but it still bugs me. Great library, though!

munawarb commented 5 years ago

Hi, Can you try grabbing the latest master branch and compiling the module on Linux again? I was using int to represent the thread ID in the C module but Python uses unsigned long to represent the thread ID, so this could be causing the overflow in your case.

munawarb commented 5 years ago

Great! I'm glad you were able to come up with an alternative. When you get a chance try testing the latest update of master and let me know how it works for you. It's bugging me as well, but reading more into how Python is handling its thread identifiers might be cluing me into a hidden problem in the Threader module that's never shown itself before. :)

On 1/28/2019 9:54 AM, georg90 wrote:

We kind off got around this issue, but it still bugs me. Great library, though!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/munawarb/Python-Kill-Thread-Extension/issues/4#issuecomment-458162382, or mute the thread https://github.com/notifications/unsubscribe-auth/AB9tJK2Jj4BGrLJe08N4pTkKKemjLWJqks5vHw8ggaJpZM4aNFph.

owneroxxor commented 5 years ago

Hi there! I am having the same issue

Python 3.7.0

    threader.killThread(self.ident)
OverflowError: signed integer is greater than maximum

Have you guys came up with a solution to this unsigned long from python Thread ident overflowing the limit on the C side?

Btw, awsome lib, thank you so much @munawarb

EDIT 1

Tried to change the INTs from C code to UNSIGNED LONGs, getting the following error to build:

threader.c: In function ‘threader_enableCancel’:
threader.c:20: warning: passing argument 2 of ‘pthread_setcanceltype’ from incompatible pointer type
/usr/include/pthread.h:494: note: expected ‘int *’ but argument is of type ‘long unsigned int *’
threader.c:22: warning: passing argument 2 of ‘pthread_setcancelstate’ from incompatible pointer type
/usr/include/pthread.h:490: note: expected ‘int *’ but argument is of type ‘long unsigned int *’

pthread_setcanceltype and pthread_setcancelstate wont accept unsigned longs, how can we contiue from here?

EDIT 2

Ok, just set to NULL the second arg of the pthread_setcanceltype and pthread_setcancelstate calls, but now the error came back:

 threader.killThread(self.ident)
OverflowError: signed integer is greater than maximum

EDIT 3

Ok, just change PyArg_ParseTuple argument "i" to "k". k means unsigned long and no errors on the build. Will test it right now ...

EDIT 4

image

It is in fact killing the threads :D loved it. But it is leaving defuncts behind... So now we have to find a way to clean those defuncts...

munawarb commented 5 years ago

Hi @owneroxxor, what architecture are you running the module on?

owneroxxor commented 5 years ago

Nvm, the defuncts are my fault. CENTOS

munawarb commented 5 years ago

Ok, thanks for the update.

munawarb commented 5 years ago

Hi @georg90 and @owneroxxor, This issue recently came up again and I did some of my own digging and found the issue. I've fixed it in #5. If you're still using this module, can you pull from Master and compile the latest code?