ok-client icon indicating copy to clipboard operation
ok-client copied to clipboard

Timeout functionality doesn't actually work

Open brianhou opened this issue 9 years ago • 12 comments

Control will return to the main thread, but the other thread with student code can continue spewing output. This output will interfere with later tests. (That's also an issue -- I think each test should print to a io.StringIO rather than to stdout.) The student thread won't die until the main thread does, when ok exits.

I don't know of a real solution to this problem that is also cross-platform. For grading purposes, we could implement timeouts with the signal module again. For students, I think it might be acceptable (since they'll kill ok once they notice it's in an infinite loop).

I might be able to take a stab at this over the weekend.

brianhou avatar Sep 10 '15 23:09 brianhou

Use subprocess instead? Something like this: http://stackoverflow.com/questions/1191374/subprocess-with-timeout

knrafto avatar Sep 12 '15 02:09 knrafto

Oh yeah! I think we didn't use it earlier because we were limiting ourselves to Python 3.2 features, but it might be promising. I'll try to investigate this weekend.

brianhou avatar Sep 13 '15 05:09 brianhou

So as far as I can tell, subprocess is just for shell things. I tried re-implementing the timeout with multiprocessing.Process rather than threading.Thread (per http://eli.thegreenplace.net/2011/08/22/how-not-to-set-a-timeout-on-a-computation-in-python), but ran into issues with shared state. It works with pure functions (and kills the process properly!), but anything that tries to mutate fails. eval and exec mutate a dictionary, so that actually breaks pretty much everything.

I think we can get away with leaving the threads as not-actually-dead as long as we can properly redirect output so they don't mix pipes.

brianhou avatar Sep 14 '15 07:09 brianhou

Maybe we can fork python3? If all we're doing is evaluating an expression.

knrafto avatar Sep 14 '15 08:09 knrafto

That's probably not a good idea. But I'm pretty sure you can use multiprocessing with impure code, but I guess eval and exec are especially bad. What dictionary do they mutate?

knrafto avatar Sep 14 '15 09:09 knrafto

By the way, I'm not sure if it's related but I get weird errors when trying to run ok tests that involve matplotlib. Here's a trace. Googling the error describes weird interactions between iPython and threading.

Test summary
    1 test cases passed! No cases failed.

Checking for software updates...
OK is up to date
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 742, in writeout_cache
    self._writeout_input_cache(conn)
  File "/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 726, in _writeout_input_cache
    (self.session_number,)+line)
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread.The object was created in thread id 4353744896 and this is thread id 140735217693440

papajohn avatar Sep 15 '15 23:09 papajohn

I ran into a similar issue when implementing sqlite3 support for ok-client. The sqlite3cursor constructor (if I remember correctly) has a parameter that, when set, will ignore the threading error. However, I'm not sure if that's possible to do here, since it seems like you aren't interacting with sqlite3 directly.

On Tue, Sep 15, 2015, 4:41 PM John DeNero [email protected] wrote:

By the way, I'm not sure if it's related but I get weird errors when trying to run ok tests that involve matplotlib. Here's a trace. Googling the error describes weird interactions between iPython and threading.

Running tests

Test summary 1 test cases passed! No cases failed.

Checking for software updates... OK is up to date Error in atexit._run_exitfuncs: Traceback (most recent call last): File "/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 742, in writeout_cache self._writeout_input_cache(conn) File "/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 726, in _writeout_input_cache (self.session_number,)+line) sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread.The object was created in thread id 4353744896 and this is thread id 140735217693440

— Reply to this email directly or view it on GitHub https://github.com/Cal-CS-61A-Staff/ok-client/issues/125#issuecomment-140581582 .

albert12132 avatar Sep 16 '15 00:09 albert12132

I spent a while trying to configure IPython to ignore threading issues, but was unsuccessful. Something like the following (when placed in the test setup) should work, but doesn't...

from IPython.core import history
from sqlite3 import connect
acc = history.HistoryAccessor()
acc.db = connect(":memory:", check_same_thread=False)

It would be nice to have an option to run everything in a single thread, perhaps by setting "--timeout 0". For lab tomorrow in DS8, we're just hiding the error output.

On Tue, Sep 15, 2015 at 5:56 PM, albert12132 [email protected] wrote:

I ran into a similar issue when implementing sqlite3 support for ok-client. The sqlite3cursor constructor (if I remember correctly) has a parameter that, when set, will ignore the threading error. However, I'm not sure if that's possible to do here, since it seems like you aren't interacting with sqlite3 directly.

On Tue, Sep 15, 2015, 4:41 PM John DeNero [email protected] wrote:

By the way, I'm not sure if it's related but I get weird errors when trying to run ok tests that involve matplotlib. Here's a trace. Googling the error describes weird interactions between iPython and threading.

Running tests

Test summary 1 test cases passed! No cases failed.

Checking for software updates... OK is up to date Error in atexit._run_exitfuncs: Traceback (most recent call last): File

"/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 742, in writeout_cache self._writeout_input_cache(conn) File

"/Users/denero/anaconda/lib/python3.4/site-packages/IPython/core/history.py", line 726, in _writeout_input_cache (self.session_number,)+line) sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread.The object was created in thread id 4353744896 and this is thread id 140735217693440

— Reply to this email directly or view it on GitHub < https://github.com/Cal-CS-61A-Staff/ok-client/issues/125#issuecomment-140581582

.

— Reply to this email directly or view it on GitHub https://github.com/Cal-CS-61A-Staff/ok-client/issues/125#issuecomment-140590131 .

papajohn avatar Sep 16 '15 04:09 papajohn

@brianhou did you ever consider using Futures for the timeout mechanism?

albert12132 avatar Dec 06 '15 18:12 albert12132

Nope, I didn't know they existed.

brianhou avatar Dec 07 '15 03:12 brianhou

Ah okay, just wondering if there was some issue with them. I'll see if we can use them to replace the timeout mechanism.

albert12132 avatar Dec 07 '15 05:12 albert12132

I took a look into Futures and it looks like it's just a wrapper around threading/multiprocessing that's basically what we're doing, so it seems likely that it'd run into the same issue. I also haven't been able to get it to kill a currently executing thread and the code seems to indicate that there probably isn't a way through the API.

I'm not sure if there's a satisfying fix to this where we actually kill threads, but we can just redirect output to an io.StringIO so there at least won't be any visual indication that the threads haven't actually been termianted.

brianhou avatar Jan 05 '16 20:01 brianhou