msgpack/msgpack-rpc — pull request #60 by authorNari (2012-07-04 04:28:13+00:00)
Title: MessagePack::AsyncResult#result is not thread-safe.

# Problem

`MessagePack::AsyncResult#result` calls `Cooio::IO#write`.

```ruby
# cool.io-1.1.0/lib/cool.io/io.rb
def write(data)
@_write_buffer << data
schedule_write
data.size
end
```

`Cooio::IO#schedule_write` in `Cooio::IO#write` may attach to `evloop` on server side.

```ruby
# cool.io-1.1.0/lib/cool.io/io.rb
def schedule_write
return unless @_io
return unless attached?
begin
enable_write_watcher
rescue IOError
end
end

def enable_write_watcher
if @_write_watcher.attached?
@_write_watcher.enable unless @_write_watcher.enabled?
else
@_write_watcher.attach(evloop) # here!!
end
end
```

If threads are not thread on server side calls `MessagePack::AsyncResult#result`, operations of above `evloop` on server are **thread-unsafe**.

In my patch, I fixed to the server side thread call `schedule_write`.

## Operations for same loop are thread-unsafe

the document of libev say:

Or put differently: calls with different loop parameters can be done in parallel from multiple threads, calls with the same loop parameter must be done serially.

[libev - a high performance full-featured event loop written in C](http://doc.dvgu.ru/devel/ev.html#threads_and_coroutines)

So we should treat carefully loop on server side...

## Does GVL save this problem?

No, it doesn't. cool.io executes a loop after GVL is released. So GVL is not related.

```c
/* cool.io-1.1.0/ext/cool.io/loop.c */
static void Coolio_Loop_ev_loop_oneshot(struct Coolio_Loop *loop_data)
{
/* Use Ruby 1.9's rb_thread_blocking_region call to make a blocking system call */
rb_thread_blocking_region(Coolio_Loop_ev_loop_oneshot_blocking, loop_data, RUBY_UBF_IO, 0);
}
```

# [FYI]Detected problem

In my environment, the following program occupies CPU with CLOSE_WAIT connections.

```ruby
require 'msgpack/rpc'

class MyHandler
def method1(params)
as = MessagePack::RPC::AsyncResult.new
Thread.new do
as.result("#{params["Message"]}")
end
return as
end
end

server = MessagePack::RPC::Server.new
server.listen("0.0.0.0", 6001, MyHandler.new)
server.run
```

In above server, CLOSE_WAIT connections on server side are remained sometime.

% netstat -an --inet 2>/dev/null | grep 6000 | grep CLOSE_WAIT
tcp 1 0 127.0.0.1:6000 127.0.0.1:60176 CLOSE_WAIT

`epoll_wait()` catches events of CLOSE_WAIT connections with each time, so the event loop has became a busy loop.

% strace -p 19583
epoll_wait(3, {{EPOLLIN, {u32=7, u64=3959959846919}}}, 64, 966) = 1
clock_gettime(CLOCK_MONOTONIC, {1194895, 432562860}) = 0
clock_gettime(CLOCK_MONOTONIC, {1194895, 432838833}) = 0
epoll_wait(3, {{EPOLLIN, {u32=7, u64=3959959846919}}}, 64, 966) = 1
clock_gettime(CLOCK_MONOTONIC, {1194896, 433319673}) = 0
clock_gettime(CLOCK_MONOTONIC, {1194896, 433572843}) = 0
epoll_wait(3, {{EPOLLIN, {u32=7, u64=3959959846919}}}, 64, 966) = 1
....
epoll_wait(3, {{EPOLLIN, {u32=7, u64=3959959846919}}}, 64, 966) = 1

Then we can see server process occupies CPU.

I think, to execute `fd_change()` and `fd_reify()` for same loop with parallel causes this problem.
If `fd_reify()` dosen't clear reify of all fd, the above problem is caused.