mark ClassHash as shareable, after all classes have been defined

[HoneyryderChuck]

So that Resource classes are usable within ractors.

There's still this function to account for, for which I didn't get the use case. Legitimate to ignore, or worth working around?

[HoneyryderChuck]

I was having a look, and there are more places which may need some ractor support fixes. Namely, DefaultResolver needs to marked as shareable. This is currently a bit difficult, given that The default resolver loads the Hosts resolver, which has a lazy_initialize logic which gets loaded on first call, so it's not really shareable-friendly.

This logic should however be fine if people instantiate their own resolver class in separate ractors.

Any recommendations to work around this?

[hanazuki]

Freezing ClassHash on load looks problematic if the user extends the library by defining a custom Resource class and registers it in ClassHash after requiring resolv. How about defining a method that make_sharable ClassHash, which can be called once the user finishes initialization?

I think making DefaultResolver sharable does not make sense because a resolver is conceptually stateful (For example, a full-featured stub resolver should keep-alive TCP connections and track network changes. It's just not yet implemented in this library). Introducing "Ractor-local default resolver" may improve usability.

[HoneyryderChuck]

Customizing a resolver should require initializing a new instance with the custom class mapping, i.e. the library should not be defining ´ClassHash` as a de-facto lazy cache. just an opinion.

think making DefaultResolver sharable does not make sense because a resolver is conceptually stateful

In this case it could work, given that TCP sockets are only used for long DNS answers, and closed afterwards. That'd be different if this were a database pool or smth of the kind, in which case a decision for a "default" would be questionable as well. Ractor-local default resolver could help as well.

[hanazuki]

Customizing a resolver should require initializing a new instance with the custom class mapping, i.e. the library should not be defining ´ClassHash` as a de-facto lazy cache.

Agreed. DNS has a few (but a growing number of) extension points that allow private/local/experimental use. So, we want some extensible extension mechanism in resolv, which may look like this (just an idea):

# resolv
class Resolv::DNS
  # Extend Resource::ClassHash & SvcParam::ClassHash to emit deprecation warning on mutation

  class ExtensionPoints
    #: (Integer, Integer) -> singleton(Resource)
    def get_resource_class(type, klass) = Resource.get_class(type, klass)
    #: (Integer) -> singleton(SvcParam)
    def get_svcparam_class(key) = SvcParam::ClassHash[key]

    # In the future, we can add more extension points:
    # def get_edns0_option_class(opt) = ...
  end

  # Opt-in for early adopters
  def self.make_shareable
    if defined?(Ractor)
      Ractor.make_shareable(Resource::ClassHash)
      Ractor.make_shareable(SvcParam::ClassHash)
    end
  end
end

# extension library
module AwesomeResolvExtension
  def get_resource_class(type, klass) = (some extension) || super
end

# user code
class MyExtensions < Resolv::DNS::ExtensionPoints
  prepend AwesomeResolvExtension
end

@my_resolver = Resolv::DNS.new(..., extensions: MyExtensions.new)
# or maybe
@my_resolver = Resolv::DNS.new(..., extensions: [AwesomeResolvExtension])

After a transition period (maybe a year or two; I'm not sure how long it should be, as resolv is an old library), we can freeze ClassHashes by default and turn Resolv::DNS.make_shareable into no-op.

Introducing Resolv::DNS.make_shareable alone is not a breaking change and easy to throw out, so I think it is a good starting point for Ractor experiments.

In this case it could work, given that TCP sockets are only used for long DNS answers, and closed afterwards. That'd be different if this were a database pool or smth of the kind, in which case a decision for a "default" would be questionable as well.

For the traditional DNS over cleartext TCP, that's right. But, for encrypted protocols such as DNS over TLS, HTTPS, or QUIC, a connection has to be reused for multiple DNS queries to amortize handshake latency and minimize computational cost (It's not very similar to DB pools because these connection-ful DNS transports allow multiplexing query-responses on a single connection).

The current DNS trend is transitioning to encryption by default, either by discovery mechanisms like DDR or opportunistic encryption with some heuristics, and I think resolv should adopt it in the future. So, IMHO, we need to design an API supporting such future enhancements before guaranteeing that DefaultResolver is Ractor-shareable.

[reesericci]

FYI, when trying to implement this change in my app, Generic tries to modify the now-frozen ClassHash at line 1774:

ClassHash[[type_value, class_value]] = c

I just commented it out for myself, but obviously that breaks some functionality

[HoneyryderChuck]

closing in favour of #62