Comments on Headius: Ruby's Thread#raise, Thread#kill, timeout.rb, and net/protocol.rb libraries are broken

No timeout library can solve this problem unless t...

2011-01-03T11:08:51.978-06:00

No timeout library can solve this problem unless they are able to guarantee they'll never fire within any ensure blocks. I don't think System Timer can make this guarantee any more than timeout.rb.

Simon, I was thinking the same thing. Just came a...

2010-06-05T15:55:43.657-05:00

Simon, I was thinking the same thing. Just came across System Timer and hoping this solves the issue!

Can anyone else comment on whether this solved it for them?

Would the SystemTimer native gem help here? http:...

2009-05-05T16:32:00.000-05:00

Would the SystemTimer native gem help here?

http://ph7spot.com/articles/system_timer

Let me correct one statement: raise and kill are n...

2009-05-05T10:47:00.000-05:00

Let me correct one statement: raise and kill are not ALWAYS bad. They come in very useful sometimes. For whatever (usually bad) reasons, you NEED them to get out of locked situations and kill a thread. Just like we people sometimes get stuck in a mindset and need someone else to come along and turn our heads to certain other truths we couldn't see from where we were; a single thread CAN get stuck. Usually the reasons are certain things beyond our control, and sometimes they actually have to do with somebody else using a bad timeout(.rb) library. ;)

But in all reality we need raise and kill. In situations where they are needed, I use Thread.raise, and just make sure I have the error rescued in the receiving thread, to cleanup whatever is needed. And of course you'd NEVER use raise or kill to stop something that should not be stopped.

raise is a normal part of programming. It's just imperative to *know* that you must be *very* careful that you have a safe environment for it before you use it, and only use it if you *have* to. It's the improper use that's dangerous - but not any more dangerous than using bad logic or math can be when working with financial data.

Here here. Timeout is totally unsafe, and causes ...

2008-03-14T10:59:00.000-05:00

Here here. Timeout is totally unsafe, and causes timeout errors to be injected "randomly" into your code--including in the middle of net/* calls, etc., which they sometimes don't know how to handle! It's disgusting! Unfortunately I can't think of easy ways around it.

The main problem is that within the ensure block it could be interrupted, I believe. And there is nothing in Ruby that allows for this not to happen.

I wonder if you could wrap the .kill calls within a mutex and maybe Thread.critical=true [which never realy works in MRI, anyway[ to avoid its problems.

I'm very glad to see this being discussed, and I h...

2008-02-27T20:47:00.000-06:00

I'm very glad to see this being discussed, and I hope JRuby, Rubinious etc take the opportunity to deprecate or fix these (IMHO) design errors in Ruby. Go ahead and break our Thread#raise-using code - its built on dodgy foundations anyway.

You can also look at http://rev.rubyforge.org/ for...

2008-02-27T12:13:00.000-06:00

You can also look at http://rev.rubyforge.org/ for non-blocking IO, which is based on libev and offer replacements for some Net/HTTP stuff.

I ran some sequential and parallel tests for WideF...

2008-02-26T23:03:00.000-06:00

I ran some sequential and parallel tests for WideFinder, in different combinations, some of which wasted threads like nobody's business. Didn't see any noticeable CPU or memory hit.

The most common trick for getting more performance out of 1.8.6 is increasing the buffer size. The second would be writing a native library.

Timeout is the failure detection mechanism in TCP/IP, and HTTP doesn't try to hide it. If you're using HTTP over TCP/IP you need to anticipate timeout errors.

To it's credit, Net::HTTP documents TimeoutError as an expected error condition on both open an read, since those use two different timeout values.

@assaf: At least you might reasonably expect to ge...

2008-02-26T21:10:00.000-06:00

@assaf: At least you might reasonably expect to get TCP errors out of net/http, since it's doing network IO. But Timeout::Error is a bit sneakier. At any rate, the mere existence of timeout.rb means you should probably rescue Timeout::Error whenever you call into a third-party library, because people use it. It simply needs to go away.

And it's not the number of threads that's a problem, it's the cost of spinning them up for every 1024 bytes. Granted, it's a lot less with green threads than with native threads, but it's still a real cost for no good reason.

"You are all rescuing Timeout::Error when you use ...

2008-02-26T20:41:00.000-06:00

"You are all rescuing Timeout::Error when you use net/http to read from a URL, right?"

Besides HTTP status codes, Net:HTTP throws a bunch of exceptions dealing with TCP issues. Either you deal with them or you don't, Timeout is one of those unspecified exceptions.

"A new thread is spun up (for every invocation, I might add)"

You'll run out of connections before you run out of threads. The 1.8.6 MRI threading implementation can do million of threads with no sweat. It also context switches in a particular way that allows this code to work, and I have more examples like this that work because of the granularity of context switching in 1.8.6.

It is a mistake to copy this code over to JRuby or 1.9 without first fixing it to work with native threads. And it's a bad idea to continue writing code like this, now that we have more deployment options.

So if you're planning on shipping code, test on both implementations to make sure it's not limited by the wrong assumptions.

@charlie, Fair point. Keep in mind though that .Ne...

2008-02-26T19:43:00.000-06:00

@charlie,

Fair point. Keep in mind though that .Net have not deprecated Thread.Abort even though it is evil, It also has a suspend which can be evil as well.

Sometimes, just before you are tearing down a process calling thread.abort may make sense, Eg. signal thread to stop, wait, thread.abort.

Also, if no unmanaged/native resources are involved it may be useful as a quick hack, if you ensure that no exceptions bubble up by catching them.

But, I think it is much too complicated for newcomers to the language, and agree that no base libraries anywhere should use this.

I hope rubinius fix up rbuf_fill before shipping 1.0

Maybe I should submit the patch that aaron mentioned

I wonder if what you've posted here is related to ...

2008-02-26T19:26:00.000-06:00

I wonder if what you've posted here is related to the WWW::Mechanize related GC bug I was hitting on Solaris, where it would segfault within a timeout block:

http://tinyurl.com/ysx2rd

Surely you could cpecify circumstances where timeo...

2008-02-26T17:55:00.000-06:00

Surely you could cpecify circumstances where timeout() is safe, for example when you're not in the presence of locks, and you can rely on the GC to clean up any resources that you allocate in the interrupted thread.

People also use timeout() as kind of a poor man's ...

2008-02-26T16:08:00.000-06:00

People also use timeout() as kind of a poor man's timed-loop-with-break sometimes, like so:

timeout(5) {sleep 1 until light.is_green}

...so that they don't have type all that Time.now boilerplate. But there are much safer ways.

No need for a separate thread when the loop has its timing needs spelled right out with the sleep() call.

One could easily slap together a quickie little class like this:

http://pastie.caboo.se/157816

...and do something like:

tick(1, 5).til {light.is_green}

Cheesy, but you get the idea.

Here is an rbuf_fill workaround I saw on ruby-talk...

2008-02-26T15:10:00.000-06:00

Here is an rbuf_fill workaround I saw on ruby-talk a while back:

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/202223

@sambo99, @paolo, and others: As you know, there a...

2008-02-26T09:00:00.000-06:00

@sambo99, @paolo, and others: As you know, there are various ways to wrap individual cases to make it mostly safe. The problem is that doing this in every case is nearly impossible, and you still have the same problems with asynchronous exceptions bubbling out to callers or through libraries that might not be prepared to handle them.

@juho: Doh. You win the prize for spotting it.

> @rbuf Is that a typo for 8192, or are you int...

2008-02-26T02:50:00.000-06:00

> @rbuf << @io.sysread(8196)

Is that a typo for 8192, or are you intentionally reading in a non-power of 2 bytes?

@turingtest: We're aware of the GHC work, and I've...

2008-02-25T20:47:00.000-06:00

@turingtest: We're aware of the GHC work, and I've pointed Charlie to the paper. Unfortunately there are a lot of additional problems when implementing it in an environment that isn't as functionally pure as Haskell, and it doesn't mesh well with external blocking calls on the JVM -- but I wouldn't entirely rule it out.

@paolo: unfortunately that approach to timeouts doesn't address all the problems with asynchronous exceptions generally, which is why @turingtest is bringing up the Haskell work (which goes a bit farther).

The timeout should probably better be handled by t...

2008-02-25T15:21:00.000-06:00

The timeout should probably better be handled by the ruby scheduling itself. timeout.rb is only useful for I/O, which is already implemented in a non-blocking way internally anyways.

Basically, changing timeout(&block) to IO#read_or_timeout_on(delay) would solve the problem. Or did I miss something ?

The same problems arise in other languages. Haske...

2008-02-25T14:15:00.000-06:00

The same problems arise in other languages. Haskell's GHC compiler uses the scheme described in Asynchronous Exceptions in Haskell.

This solves the issue by adding two new (scoping) primitives: block and unblock. These nest, and the innermost controls whether raised exceptions are noticed (unblock) or not (block). When a blocked thread receives an exception it queues the exception until the next moment it becomes unblocked. Unblocked threads may notice exceptions at any time.

This makes it possible to write a timeout library, though it is still subtle. The haskell library is control-timeout.

The latest ghc version has spawned threads inherit the block/unblock status of the parent thread. This is particularly useful for starting threads in a blocked state.

Sorry I cannot provide code in Ruby. But there is...

2008-02-25T10:54:00.000-06:00

Sorry I cannot provide code in Ruby. But there is a way to make it work, and it is to run a single high-priority thread to do the delays, and schedule/unschedule timeouts in the same high-priority thread.

It's how both Squeak and GNU Smalltalk do it.

This is a very interesting post! I come from a win...

2008-02-25T07:09:00.000-06:00

This is a very interesting post! I come from a windows background and have seen this kind of stuff mess up in production. In general the pattern I tended to use when I needed a timeout was WaitForMultipleObjects and make sure I would wait for a terminate event. Thread.Abort is always a big no-no that you only use when you have no choice.

I wonder if something along these lines would partially fix it? I still dislike the thread.raise stuff its ugly

def timeout(sec, exception=Error)
return yield if sec == nil or sec.zero?
raise ThreadError, "timeout within critical session" if Thread.critical
begin
terminate_event = Event.new
x = Thread.current
y = Thread.start {
result = WaitForSingleObject(terminate_event, timeout)

x.raise exception, "execution expired" if x.alive? && result == TIMEOUT
}
yield sec
# return true
ensure
if y and y.alive?
terminate_event.signal
end
end
end

but the trick is that I do not think Ruby has a WaitForSingleObject

@headius: Amen. I was bitten by the same problems....

2008-02-25T03:47:00.000-06:00

@headius: Amen. I was bitten by the same problems. When using Thread#kill you easily start thinking "Oh, I could spawn another thread that kills the first one if it take too long or something". Then you get some unpredictable results and can also easily think "ok, if I sleep that process for 2 sec. it seems to work". And then it gets worser. Thanks for bringing visibility to that problem.

@martin probst: Comparing to unix processes, Thread#kill is like `kill -9`. You'd better only use it on highly exceptional cases. In the same way, long running software in C all have a main loop that handles the various signals. Do the same in your ruby "process" and you'll be fine.

@all: DRb is also highly flawed in that sense. Every proxied call can be broken by a Thread#kill.

The thing is that this kind of kill/raise function...

2008-02-25T02:14:00.000-06:00

The thing is that this kind of kill/raise functionality is highly desirable. E.g. in every modern application server you'll be running several applications in one process, handled by multiple threads.

Now if one of these applications has a coding error, it will destroy the whole server. If one application has a denial of service problem, where it goes into an unterminated loop, you can easily kill the whole appserver.

And you certainly will have such issues in large code bases. At least in some library you're using.

So I think we really need some kind of a solution for this problem. Any ideas?

Well we could perhaps do...main = Thread.currentti...

2008-02-25T01:02:00.000-06:00

Well we could perhaps do...

main = Thread.current
timer = Thread.new { sleep 5; raise }
begin
begin
lock(some_resource)
do_some_work
ensure
timer.kill
end
ensure
unlock_some_resource
end

This is mostly tongue-in-cheek though. I'd agree a better solution is needed