Disable reloading only for stateful vars

[dsteurer]

I have been getting somewhat erratic errors with this library that I am not able to reproduce easily. The integrant.repl/resume function raised exception due to Ref instances not being resolved. At the same time integrant.core/resume applied to the same system worked without errors. I suspect that the error arises when the integrant.core is reloaded because reloading is disabled for integrant.repl. What seemed to fix the issue was to move the stateful vars ('system', ...) into a separate namespace and disable reloading only for that namespace. Unfortunately I don't really know why (or if) it works.

[weavejester]

If you get the intermittent errors again, can you post a stack trace or more details about the error? Also, what version of Integrant are you using?

I'll keep the PR open for now, but I can't merge changes without an explanation of why, particularly since moving the vars into a separate namespace should make no difference (in theory!).

[dsteurer]

Note that the change that is supposed to make a difference here is removing (repl/disable-reload!) from the integrant.repl namespace.

I understand that the changes can't be merged at this point. Unfortunately I am not that familiar with clojure's reloading semantics. I seem to be able to reproduce the error now and will try to create a minimal example. (Maybe the bug lies somewhere else.)

[weavejester]

A reproducible example would be fantastic. It doesn't even need to be 100% reproducible, so long as it will error within a reasonable number of trials.

[dsteurer]

I think this gist illustrates the problem. I can reliably reproduce the error only if the sources of integrant.core and integrant.repl are in the same project. But I saw these kinds of errors also when they are project dependencies.

[weavejester]

Thanks for trying to reproduce this.

In your example project, the reason for its odd behavior is because the integrant.core.Ref record is redefined, but the prep function returns a data structure that references the old integrant.core.Ref record. This means that integrant.core/ref? returns false when checking if ::a contains a reference.

I'm not sure what the best fix for this is. I'll give it some thought and report back.

[dsteurer]

The issue is that I don't seem to be able to get back into a consistent state. (Even after calling igr/set-prep! or igr/clear.) The behavior seems very strange to me. For example, calling (ig/init igr/config) gives the expected result, while (igr/go) fails to resolve the ref. So somehow igr/go seems to call a previous version of ig/init or something like that. I have updated the gist to illustrate this behavior.

[weavejester]

Ah, I see what you mean. I still think the root cause of the behaviour is the change in the Ref record when the integrant.core namespace is reloaded, but there's also something else going on around integrant.repl.

I did some digging, and it looks like integrant.repl is holding onto an old version of the integrant.core namespace. This problem persists across Clojure 1.7, 1.8 and 1.9.

I altered integrant.repl/init to print out the reference to the integrant.core/init function in various ways:

(defn init []
  (prn ig/init)
  (prn (var-get #'ig/init))
  (prn (var-get (find-var `ig/init)))
  (alter-var-root #'system (fn [sys] (halt-system sys) (ig/init config)))
  :initiated)

And weirdly, even the direct var reference didn't work. Only find-var did:

reloadbug=> (igr/init)
#object[integrant.core$init 0x5164bfba "integrant.core$init@5164bfba"]
#object[integrant.core$init 0x5164bfba "integrant.core$init@5164bfba"]
#object[integrant.core$init 0x373a4cbe "integrant.core$init@373a4cbe"]
:reloadbug/a :halt
:reloadbug/b :halt
:reloadbug/b :init
:reloadbug/a :init #integrant.core.Ref{:key :reloadbug/b}
false
:initiated
reloadbug=> ig/init
#object[integrant.core$init 0x373a4cbe "integrant.core$init@373a4cbe"]

I knew that Clojure could hold onto old references of functions, but I didn't think it was possible for Clojure to hold onto old references of vars. My guess is that this occurs because the refresh function removes vars in a namespace, but the references to those old vars persists in the untouched integrant.repl namespace.

As far as I can tell, this will only occur if integrant.core is one of the namespaces that is refreshed, either because it's been directly included in the source tree, or via checkouts. I don't think there's any other way integrant.core can be caught up in refresh, but I might be wrong.

Let me think about the solution. I think I'm leaning toward an integrant.repl.state namespace, but I'm not sure.

[weavejester]

I've given this some thought, and I think the integrant.repl.state namespace is probably the best solution.

If the integrant.core namespace is refreshed via clojure.tools.namespace.repl/refresh, then the vars are removed and replaced, including the Ref record. However, becauseintegrant.repl is not included in the refresh, it hangs onto the old vars and the old record.

Moving the (disable-reload!) to a smaller namespace that doesn't reference integrant.core is probably the neatest way of solving this edge case.

[weavejester]

Released as [integrant/repl "0.2.0"]. Thanks for the patch and the investigation of this issue!