Variable scoping

[Brian Candler]

I've been away from this list for a while, so I don't know what the current
thinking is behind the scoping rules for block-local variables, and whether
they're likely to be changed.

But I've just been bitten by this. Have a look at the following code, and
see if you can see what's wrong with it just by inspection. It accepts TCP
connections on a socket, and starts a server in a thread for each one. It's
a nice pattern for TCP servers.

-----------------------------------------------------------------------
require 'socket'
module MyModule
def run
puts "Hello, world!"
sleep 10
puts "Goodbye"
# ... could do other stuff
end
end

port = (ARGV[0] || 7000).to_i
bind = (ARGV[1] || '0.0.0.0')
server = TCPserver.new(bind, port)

# For each connection, add our 'run' method to the I/O object and run it
# in its own thread

while (session = server.accept)
session.extend MyModule
Thread.new do
begin
session.run
rescue Exception => e
STDERR.puts "Caught exception: #{e}\n\t#{e.backtrace.join("\n\t")}"
ensure
session.close
end
end
end
-----------------------------------------------------------------------

To demonstrate the problem: run it in one window, and then in two further
windows type "telnet localhost 7000" in each, less than 10 seconds apart.
Watch the second one fail, and also see an exception reported by the server.

After quite a bit of head-scratching, and boiling my server down to the
simple code shown above, I managed to work out what was wrong and fix it.

However, I wonder if the language could have helped me more here? "ruby -w"
didn't spot any problem (although perhaps it would be hard for it to do so).

I think this is a type of problem which unit testing is unlikely to find,
unless you are specifically aware of it.

Regards,

Brian.


v




v




v




v




v




The fix:
-----------------------------------------------------------------------
...
while (s = server.accept)
s.extend MyModule
Thread.new(s) do |session|
begin
...

 

[ts]

B> But I've just been bitten by this. Have a look at the following code, and
B> see if you can see what's wrong with it just by inspection. It accepts TCP
B> connections on a socket, and starts a server in a thread for each one. It's
B> a nice pattern for TCP servers.

This is because you have never read ruby-man-1.4, and it's very bad

The example given is

------------------------------------------------------------
Even shorter using thread:

require "socket"

gs = TCPserver.open(0)
addr = gs.addr
addr.shift
printf("server is on %d\n", addr.join(":"))

while TRUE
ns = gs.accept
print(ns, " is accepted\n")
Thread.start do
s = ns # save to dynamic variable
while s.gets
s.write($_)
end
print(s, " is gone\n")
s.close
end
end

 

[Brian Candler]

This is because you have never read ruby-man-1.4, and it's very bad

You mean very bad of me not to read it? Fair enough. I _have_ read quite a
lot though Besides, we can all make mistakes.

| while TRUE
| ns = gs.accept
| print(ns, " is accepted\n")
| Thread.start do
| s = ns # save to dynamic variable

But isn't there a race there? That is, if the interpreter context-switches
after the Thread.start, goes back around the loop, and ns is assigned to a
new object before "s = ns" is executed.

I think for safety you should do

Thread.start(ns) do |s|

But that's the trouble with this thread-safety stuff. Maybe 999 out of 1000
times the previous code would work. It's almost certainly not possible to
catch that sort of problem with unit-testing, unless you splatter "sleep"s
all over the code when testing.

Some sort of static indication of a possible problem would be very useful.

It would have to be able to tell that:
- you are using a variable within a Thread .... block
- the variable may be assigned to again by the other thread
(which, in this case, is an *earlier* assignment because it's in a loop)

Regards,

Brian.

 

[Ara.T.Howard]

You mean very bad of me not to read it? Fair enough. I _have_ read quite a
lot though Besides, we can all make mistakes.

well - not guy really ;-)

| while TRUE
| ns = gs.accept
| print(ns, " is accepted\n")
| Thread.start do
| s = ns # save to dynamic variable

But isn't there a race there? That is, if the interpreter context-switches
after the Thread.start, goes back around the loop, and ns is assigned to a
new object before "s = ns" is executed.

I think for safety you should do

Thread.start(ns) do |s|

But that's the trouble with this thread-safety stuff. Maybe 999 out of 1000
times the previous code would work. It's almost certainly not possible to
catch that sort of problem with unit-testing, unless you splatter "sleep"s
all over the code when testing.

Some sort of static indication of a possible problem would be very useful.

It would have to be able to tell that:
- you are using a variable within a Thread .... block
- the variable may be assigned to again by the other thread
(which, in this case, is an *earlier* assignment because it's in a loop)

Regards,

Brian.

why not fork? it's not that expensive anymore and would solve most of your
problems.

-a
--
===============================================================================
| EMAIL :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE :: 303.497.6469
| A flower falls, even though we love it;
| and a weed grows, even though we do not love it.
| --Dogen
===============================================================================

 

[Brian Candler]

why not fork? it's not that expensive anymore and would solve most of your
problems.

It solves the wrong problem. My server has a pool of objects, and the
incoming connections need to be able to see/modify them. Using fork, you
lose all ability to share data without using some sort of IPC.

I'm quite happy to use mutexes to make the accesses to objects thread-safe;
what caught me out is that a *local variable* is not thread safe. i.e. the
objects were fine, I just ended up sending a message to the wrong object!

Cheers,

Brian.