#1114 ✓resolved

Repeating access to a nil OneToOne association causes runtime exception

Reported by MarkMT | October 28th, 2009 @ 04:17 PM | in 0.10.2

I have two models related by a one to one relationship. If I have an instance of the parent model with no associated child object, attempting to access the child association should return nil.

What I observe is that accessing the child association does return nil on first attempt, but if I repeat the operation, a runtime exception occurs.

Details here - http://gist.github.com/220838

Results generated use dm-core next -


Comments and changes to this ticket

  • MarkMT

    MarkMT October 29th, 2009 @ 09:27 AM

    I don't have a very deep knowledge of DM internals, but I've been looking through the code and my assessment of what is happening is the following...

    The association reader (User#address in my case) invokes Associations::OneToOne::Relationship#get which executes -

    relationship.get(source, other_query).first

    'relationship' references a OneToMany relationship embedded in the OneToOne relationship. 'relationship.get' returns a collection, which would ordinarily have one element, and 'first' ensures that only that element is returned.

    In the case I described, because there is no child 'address' object associated with the parent 'user', relationship.get returns an empty collection, []. It is when 'first' is called on this result that the problem I've observed occurs.

    Collection#first includes this snippet...

      collection = if !with_query && (loaded? || lazy_possible?(head, query.limit))
        new_collection(query, super(query.limit))

    In the condition expression, !with_query is true, and the first time the association is accessed, both loaded? and lazy_possible?(head, query.limit) are false. However it turns out that the second time the association is accessed, lazy_possible? is true (I haven't dug into that to understand why) and this causes new_collection(query, super(query.limit)) to be executed.

    query.limit is equal to 1. super(query.limit) invokes LazyArray#first on the empty collection [], and it turns out that this returns [nil]. This gets passed into new_collection as the parameter 'resources' :

      def new_collection(query, resources = nil, &block)
        collection = self.class.new(query, &block)
        if resources

    The critical point is that within this method, collection.set(resources) will be called if resources is non-nil. In this case, resources is [nil] and so collection.set will be called. However it turns out that inside the implementation of Collection#set, when resources is an Enumerable, its elements are expected to be non-nil, as evidenced by a later attempt to call read_only? in One_to_Many::Collection#inverse_set.

    I don't know enough about how DM is supposed to work to know what the right solution is, but one simple fix is to just modify new_collection above, replacing -

        if resources


        if resources && resources.all?
  • MarkMT

    MarkMT October 30th, 2009 @ 08:51 AM

    I've run into another situation that also fails, with similar, though not identical, behavior but the same underlying cause. This one makes me think that there is a better fix than the one I suggested above that will enable both situations to be addressed in more or less the same way...

    Here is a script that illustrates the second situation - http://gist.github.com/222295

    The problem occurs when I assign a new child object to a parent that initially has no child, and do so using the ||= operator, i.e. equivalent to -

    user.address = user.address || Address.new

    ...so whereas in the case I reported earlier there were two consecutive calls to the association reader, here there is a call to the reader followed by one to the writer.

    In this case a runtime error occurs on the call to the writer. The writer method calls Associations::OneToOne::Relationship#set , which in turn calls Associations::OneToMany::Relationship#set on the OneToMany relationship embedded in the OneToOne relationship. This method executes -


    It turns out that in the particular situation I've described, get!(source) returns a one element array, [nil] (internally this seems to be returned by instance_variable_set, but I haven't dug further to understand why this only happens when the writer is called after the reader).

    Calling 'replace' on this particular value is what causes the exception. This leads (indirectly, via super) to a call to Collection#resources_removed, which executes

         resources.each { |resource| resource_removed(resource) }

    where resource_removed is defined in the subclass OneToMany::Collection. Since 'resources' is [nil], the loop gets called once for the nil element. resource_removed calls inverse_set(resource,nil). 'resource' appears in inverse_set as a local variable 'source' and internally the method performs a test

         if source.read_only?

    Right here is where the exception occurs, because you can't call read_only on nil.

    This bears a very close resemblance to the problem I reported initially. As I noted earlier, the problem occurs in that case when Collection#set(resources) is called with the argument [nil]. In that case the implementation of 'set' calls Collection#resources_added, which executes

         resources.each { |resource| resource_added(resource) }

    where resource_added is defined in the subclass OneToMany::Collection. This an exact parallel to the case above with resources_removed, since like resource_removed, resource_add also calls OneToMany::Collection#inverse_set with a nil resource and fails when attempting to call read_only? on it.

    The call to read_only? was added in a patch about a week ago -


    replacing a previous call to 'frozen?', which didn't have this problem because frozen? is defined on nil.

    Although the two cases I've described are distinct, they both stem from the same underlying cause and manifest themselves in different but similar ways. In the first case I previously suggested intercepting the [nil] argument before calling Collection#set. This solution doesn't apply so readily in the second case. A better solution imo would be to apply parallel fixes for both problems in the two methods resources_added and resources_removed respectively. This can be done simply by replacing

         resources.each { |resource| resource_added(resource) }


         resources.compact.each { |resource| resource_added(resource) }

    in Collection#resources_added and replacing

         resources.each { |resource| resource_removed(resource) }


         resources.compact.each { |resource| resource_removed(resource) }

    in Collection#resources_removed.

  • MarkMT

    MarkMT October 30th, 2009 @ 08:58 AM

    Sorry, there's a copy and paste error above... Collection#resources_added actually does this -

        resources.map { |resource| resource_added(resource) }

    not this

        resources.each { |resource| resource_added(resource) }

    and so the proposed solution for the first case is...

        resources.compact.map { |resource| resource_added(resource) }
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) October 30th, 2009 @ 04:07 PM

    • State changed from “new” to “unconfirmed”

    The actual bug is that the resources Array contains a nil at all. A nil value is not permitted inside a Collection, and the resources Array is being used to populate the Collection, so it shouldn't include nil either.

    What I'd suggest doing is checking at the "entry point" and see why the resources Array contains a nil.

  • MarkMT

    MarkMT November 1st, 2009 @ 11:41 PM

    I believe the root problem is in Collection#first...

    This commit - http://github.com/datamapper/dm-core/commit/6a6add48ca6f5dcf85af7ae...

    a few days ago introduced the following expression for the return value that applies the first time OneToMany::Collection#first is called -

        head[0] = collection.to_a.first

    In the case where no child object exists, collection.to_a is an empty array [], and [].first returns nil, which gets stored in @head. A subsequent call to super(query.limit) the next time the association is accessed returns @head.first(1), which is the non-empty array [nil] that is passed as the initial resources array to new_collection.

    A subsequent call to lazy_load also transfers the nil value into the collection's @array variable.

    I'm not sure what the ultimate solution is, but backing out of that patch has resolved the issues I was observing.

  • Dirkjan Bussink

    Dirkjan Bussink November 2nd, 2009 @ 02:20 PM

    Could you check whether your issue is fixed with this commit:


    Also added a spec for this case so it won't happen again.

  • MarkMT

    MarkMT November 5th, 2009 @ 02:38 PM

    Thanks Dirkjan. Sorry it's taken me a few days to circle back to this issue. I can confirm that that commit fixes both manifestations of the problem I described.

    FYI I also discovered a case in which simply backing out of the previous patch as I described exposed another problem (presumably related to the one that patch was designed to fix). Your commit does not suffer from this problem, so seems safe.

    BTW, I guess reflecting the limits of my ruby experience it took me a while to realize what happens when none of the conditions in the if construct are satisfied, given that you no longer have a closing else clause. Eventually I figured out that an if statement without a matched condition actually evaluates to nil, which of course is part of why this works.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) November 6th, 2009 @ 01:41 AM

    • State changed from “unconfirmed” to “resolved”
    • Milestone set to 0.10.2
    • Assigned user set to “Dirkjan Bussink”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »