Re: Make eLauncherAuthor thread-safe [patch]

On Wed, 2010-03-10 at 14:34 +0000, Thomas Leonard wrote:
> On 10 March 2010 14:10, Kevin Reid <[email protected]> wrote:
> > On Mar 10, 2010, at 8:49, Thomas Leonard wrote:
> >
> >> There are three commits. The first adds a VatSeeder Java class, which
> >> can be passed to Vat.seed and evaluates its twine in privileged scope.
> >> This is needed because safeScope is not thread-safe, so it is never OK
> >> to pass an E object here (as eLauncherAuthor does). It also changes
> >> seedVatAuthor to use the new system:
> >>
> >>  http://gitorious.org/~tal-itinnov/repo-roscidus/it-innovation/commit/8240992d252cdf65311113a6fd910ad468c1e3c8
> >
> > Why not use seedVatAuthor rather than adding any Java code? It was our
> > intent that seedVatAuthor is a sufficiently safe mechanism. Does it
> > fail to be?
> 
> It's possible that it is thread-safe, but I'd be surprised. It's very
> fragile. For example, it started:
> 
>         to run(vat :Vat, privSrc :String) :rcvr {
>             return vat.seed(fn{
>                 def eParser := <elang:syntax.makeEParser>
> 
> If makeEParser was an emaker, this would certainly be unsafe
> (ImportLoader isn't thread-safe for E code).
> If it bound it using =~, it would be unsafe (the lazy slot used by the
> __matchBind expansion isn't safe).
> If it tried to verify the result with require(eParser != null) it
> would be unsafe.
> If the "auths" map returned by runeAuthor wasn't thread-safe, it would
> be unsafe too.
> etc
> 
> It seemed like verifying every line of code used for unsafe things was
> harder than replacing it with a bit of Java.

Any thoughts on this? The second and third patches in the series (making
eLauncherAuthor use seedVat) certainly make a difference, as the old
code is clearly wrong. This is easy to see, by adding a delay to the
start of esrc/org/erights/e/elang/expand/__matchBind.emaker:

pragma.syntax("0.9")
for x in 1..1000 {
}

Then:

$ rune
# problem: Failed: not synchronously callable

However, I think that the first patch (Java VatSeeder) might also allow
some simplifications in the rest of the code. For example, the locking
in ImportLoader and LazyEvalSlot could probably go away. It seems silly
to put effort into making them thread-safe when the thread safely is
only used for a few lines of code executing briefly in seedVatAuthor.

http://gitorious.org/~tal-itinnov/repo-roscidus/it-innovation/commits/threading


-- 
Dr Thomas Leonard
IT Innovation Centre
2 Venture Road
Southampton
Hampshire SO16 7NP

Tel: +44 0 23 8076 0834
Fax: +44 0 23 8076 0833
mailto:tal-v5nx5w6akNyLE8xUarVfuPLx9OUvmyODWmv/[email protected]
http://www.it-innovation.soton.ac.uk