Code Review: WEBrick

tfpt review “/shelveset:WEBrick;REDMOND\tomat”

Implements IO.select, String#to_f, Thread#kill, Thread#start,
Thread#inspect, fixes TCPServer, File::Constants, and some other minor
fixes.

Tomas

Hi Tomas,

What happens if someone calls TCPServer.Accept() without calling
TCPServer#CreateReadWaitHandle() first?

[The new implementation of Accept method needs a WaitHandle to have been
created. It seems that the only time this occurs is when RubyIO.Select()
is
called.]

Pete

I forgot to check the result, will fix that.
Thanks for finding this.

Tomas

Hi Tomas,

I noticed in the diff for BasicSocket.cs that you added a TODO comment,
saying that static variables cannot be used (the variable in question
was:
private static bool _doNotReverseLookup;).

I had wondered about this and was not sure how to handle it. Should I
have
registered a singleton variable instead? What is actually the problem
with
using a static value? Is it that the sharing of singleton variables has
different thread semantics to .NET static variables?

Cheers,
Pete

We need to run multiple isolated IronRuby engines in the same
app-domain. Entire state that the runtime and libraries need should be
per RubyExecutionContext, which can be retrieved by calling
RubyUtils.GetExecutionContext(CodeContext) in library methods. Currently
you need to create e.g. a hash from RubyExecutionContext -> data that
would be thread safe and static. We’ll probably make it easier for
libraries and will provide a hash on RubyExecutionContext where you can
put stuff.

You don’t need to change it now, you can wait for the hash to be
available. Correct functionality of the socket library and tests are
more important now.

Tomas

So what you are saying is the RubyExecutionContext in between threads
and
AppDomains when it comes to scope?

We can’t use statics because they are scoped to AppDomains and there may
be
more than one RubyExecutionContext in a single AppDomain.

We can’t use [ThreadStatic] attributes on our statics, to limit their
scope
to a single Thread, because there can be multiple threads in a
RubyExecutionContext and we need the static value to be shared amongst
all
the threads in the RubyExecutionContext.

Is that right?

Pete

Yes.

Tomas