Validator is not thread safe

[dekellum]

In a threaded environment we need to synchronize around all calls to Validator.validate!(). Otherwise we get bogus errors like the one shown below. In this case, the referenced file does indeed contain complexText; as it does when successfully run single threaded.

Why are we using threads?: Threaded application server (rack, sinatra on jruby), and trying to validate json against a schema just after it is produced but before it is returned.

I suspect this is due to use of @@class_vars for state in Validator, but I still don't quite understanding it well enough to know if would be reasonable to remove those for instance variables, that would avoid the issue. How might I best help on this? Can I offer a test case that demonstrates the problem to start with?

JSON::Schema::ValidationError: The referenced schema 'file:///home/david/src/evri/evrinid/content-store/model/schema/common.json#complexText' cannot be found in schema file:///home/david/src/evri/evrinid/content-store/model/schema/content.json#
    /home/david/src/json-schema/lib/json-schema/validator.rb:41:in `validation_error'
    /home/david/src/json-schema/lib/json-schema/attributes/ref.rb:50:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:70:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/validator.rb:68:in `validate'
    /home/david/src/json-schema/lib/json-schema/schema.rb:38:in `validate'
    /home/david/src/json-schema/lib/json-schema/attributes/properties.rb:15:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/attributes/properties.rb:6:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:70:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/validator.rb:68:in `validate'
    /home/david/src/json-schema/lib/json-schema/schema.rb:38:in `validate'
    /home/david/src/json-schema/lib/json-schema/attributes/items.rb:10:in `validate'
    org/jruby/RubyArray.java:1612:in `each'
    org/jruby/RubyEnumerable.java:951:in `each_with_index'
    /home/david/src/json-schema/lib/json-schema/attributes/items.rb:7:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:70:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/validator.rb:68:in `validate'
    /home/david/src/json-schema/lib/json-schema/schema.rb:38:in `validate'
    /home/david/src/json-schema/lib/json-schema/attributes/properties.rb:15:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/attributes/properties.rb:6:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:70:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/validator.rb:68:in `validate'
    /home/david/src/json-schema/lib/json-schema/schema.rb:38:in `validate'
    /home/david/src/json-schema/lib/json-schema/attributes/ref.rb:47:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:70:in `validate'
    org/jruby/RubyHash.java:1170:in `each'
    /home/david/src/json-schema/lib/json-schema/validator.rb:68:in `validate'
    /home/david/src/json-schema/lib/json-schema/schema.rb:38:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:140:in `validate'
    /home/david/src/json-schema/lib/json-schema/validator.rb:258:in `validate!'

[hoxworth]

Agreed, the validator is not thread safe. I'll work on extracting the class variables into something more thread safe to hopefully alleviate these problems in the future.

[hoxworth]

This issue in particular is going to take a bit of time to resolve, as much of the validator revolves around utilizing class variables currently. I'll be getting to it, but given the change will affect a lot of aspects I'm going to take a little time to be careful about not breaking anything else.

[dekellum]

That sounds perfectly reasonable to me and thanks for your efforts. Until then, we put a synchronize block around calls to the validator. So there is an, albeit performance limiting, work around. Hmmm... actually, if you think it going to take a while; would you accept a patch to move the lock into the Validator class itself? This way it won't be bite anyone else by surprise, and the lock can be removed when you are able to convert class to instance variables.

[hoxworth]

I'll look into adding it into the Validator class; this shouldn't hurt anything anyone else is doing.

[myronmarston]

We've been bitten by this, too :(. There's a lot of mutable global state in the validator (in the class variables) that's causing the problem.

I think it would be best to refactor things so no access to global mutable state is needed...but in the meantime, here's a simpler suggestion:

• Expose each class variable as a (private) method. (Private in the sense that it's not part of json-schema's documented public API; but it may in fact be public in the ruby sense).
• Change each place that references the class variables directly to use the new methods.
• Change the methods to use a thread local variable.

No mutex would be needed then. It would change the global state to be global to the thread rather than global to the process.

[hoxworth]

The 1.1.0 patch removes most of the global state that is mutable during validation, and a mutex lock has been added around the rest.

I need to take some time to refactor the majority of the validation core, as it truly is a mess. Hopefully I'll get around to that in the next couple of weeks...

[myronmarston]

👏