clj-commons / byte-streams

A Rosetta stone for JVM byte representations
417 stars 33 forks source link

Unable to load byte-streams ns more than once #26

Closed jeaye closed 7 years ago

jeaye commented 7 years ago

Here's an example repl session (from a fresh clone of byte-streams, with no other code):

$ lein repl
nREPL server started on port 46803 on host 127.0.0.1 - nrepl://127.0.0.1:46803
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.8.0
OpenJDK 64-Bit Server VM 1.8.0_121-b13
Reflection warning, /home/jeaye/projects/lets-bet/byte-streams/target/9ebf76f7ebb52177860c3ac205bfc4727d674f41-init.clj:1:1589 - reference to field ns can't be resolved.
Reflection warning, /home/jeaye/projects/lets-bet/byte-streams/target/9ebf76f7ebb52177860c3ac205bfc4727d674f41-init.clj:1:3847 - reference to field name can't be resolved.
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (use 'byte-streams :reload-all)
WARNING: vector-of already refers to: #'clojure.core/vector-of in namespace: user, being replaced by: #'byte-streams/vector-of
nil
user=> (use 'byte-streams :reload-all)

user=>      java.lang.IllegalArgumentException: No implementation of method: :assoc-conversion of protocol: #'byte-streams.graph/IConversionGraph found for class: byte_streams.graph.ConversionGraph
clojure.lang.Compiler$CompilerException: java.lang.IllegalArgumentException: No implementation of method: :assoc-conversion of protocol: #'byte-streams.graph/IConversionGraph found for class: byte_streams.graph.ConversionGraph, compiling:(byte_streams.clj:334:1)

user=> *e
#error {
 :cause "No implementation of method: :assoc-conversion of protocol: #'byte-streams.graph/IConversionGraph found for class: byte_streams.graph.ConversionGraph"
 :via
 [{:type clojure.lang.Compiler$CompilerException
   :message "java.lang.IllegalArgumentException: No implementation of method: :assoc-conversion of protocol: #'byte-streams.graph/IConversionGraph found for class: byte_streams.graph.ConversionGraph, compiling:(byte_streams.clj:334:1)"
   :at [clojure.lang.Compiler load "Compiler.java" 7391]}
  {:type java.lang.IllegalArgumentException
   :message "No implementation of method: :assoc-conversion of protocol: #'byte-streams.graph/IConversionGraph found for class: byte_streams.graph.ConversionGraph"
   :at [clojure.core$_cache_protocol_fn invokeStatic "core_deftype.clj" 568]}]
 :trace
 [[clojure.core$_cache_protocol_fn invokeStatic "core_deftype.clj" 568]
  [clojure.core$_cache_protocol_fn invoke "core_deftype.clj" 560]
  [byte_streams.graph$eval21215$fn__21240$G__21196__21253 invoke "graph.clj" 91]
  [clojure.lang.AFn applyToHelper "AFn.java" 171]
  [clojure.lang.AFn applyTo "AFn.java" 144]
  [clojure.lang.Atom swap "Atom.java" 79]
  [clojure.core$swap_BANG_ invokeStatic "core.clj" 2263]
  [clojure.core$swap_BANG_ doInvoke "core.clj" 2253]
  [clojure.lang.RestFn invoke "RestFn.java" 529]
  [byte_streams$eval22066 invokeStatic "byte_streams.clj" 336]
  [byte_streams$eval22066 invoke "byte_streams.clj" 336]
  [clojure.lang.Compiler eval "Compiler.java" 6927]
  [clojure.lang.Compiler load "Compiler.java" 7379]
  [clojure.lang.RT loadResourceScript "RT.java" 372]
  [clojure.lang.RT loadResourceScript "RT.java" 363]
  [clojure.lang.RT load "RT.java" 453]
  [clojure.lang.RT load "RT.java" 419]
  [clojure.core$load$fn__5677 invoke "core.clj" 5893]
  [clojure.core$load invokeStatic "core.clj" 5892]
  [clojure.core$load doInvoke "core.clj" 5876]
  [clojure.lang.RestFn invoke "RestFn.java" 408]
  [clojure.core$load_one invokeStatic "core.clj" 5697]
  [clojure.core$load_all$fn__5618$fn__5621 invoke "core.clj" 5713]
  [clojure.core$load_all$fn__5618 invoke "core.clj" 5713]
  [clojure.lang.AFn call "AFn.java" 18]
  [clojure.lang.LockingTransaction run "LockingTransaction.java" 273]
  [clojure.lang.LockingTransaction runInTransaction "LockingTransaction.java" 229]
  [clojure.core$load_all invokeStatic "core.clj" 5711]
  [clojure.core$load_all invoke "core.clj" 5705]
  [clojure.core$load_lib$fn__5626 invoke "core.clj" 5737]
  [clojure.core$load_lib invokeStatic "core.clj" 5736]
  [clojure.core$load_lib doInvoke "core.clj" 5717]
  [clojure.lang.RestFn applyTo "RestFn.java" 142]
  [clojure.core$apply invokeStatic "core.clj" 648]
  [clojure.core$load_libs invokeStatic "core.clj" 5774]
  [clojure.core$load_libs doInvoke "core.clj" 5758]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 650]
  [clojure.core$use invokeStatic "core.clj" 5860]
  [clojure.core$use doInvoke "core.clj" 5860]
  [clojure.lang.RestFn invoke "RestFn.java" 421]
  [user$eval18373 invokeStatic "9ebf76f7ebb52177860c3ac205bfc4727d674f41-init.clj" 1]
  [user$eval18373 invoke "9ebf76f7ebb52177860c3ac205bfc4727d674f41-init.clj" 1]
  [clojure.lang.Compiler eval "Compiler.java" 6927]
  [clojure.lang.Compiler eval "Compiler.java" 6890]
  [clojure.core$eval invokeStatic "core.clj" 3105]
  [clojure.core$eval invoke "core.clj" 3101]
  [clojure.main$repl$read_eval_print__7408$fn__7411 invoke "main.clj" 240]
  [clojure.main$repl$read_eval_print__7408 invoke "main.clj" 240]
  [clojure.main$repl$fn__7417 invoke "main.clj" 258]
  [clojure.main$repl invokeStatic "main.clj" 258]
  [clojure.main$repl doInvoke "main.clj" 174]
  [clojure.lang.RestFn invoke "RestFn.java" 1523]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__941 invoke "interruptible_eval.clj" 87]
  [clojure.lang.AFn applyToHelper "AFn.java" 152]
  [clojure.lang.AFn applyTo "AFn.java" 144]
  [clojure.core$apply invokeStatic "core.clj" 646]
  [clojure.core$with_bindings_STAR_ invokeStatic "core.clj" 1881]
  [clojure.core$with_bindings_STAR_ doInvoke "core.clj" 1881]
  [clojure.lang.RestFn invoke "RestFn.java" 425]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate invokeStatic "interruptible_eval.clj" 85]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate invoke "interruptible_eval.clj" 55]
  [clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__986$fn__989 invoke "interruptible_eval.clj" 222]
  [clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__981 invoke "interruptible_eval.clj" 190]
  [clojure.lang.AFn run "AFn.java" 22]
  [java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1142]
  [java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 617]
  [java.lang.Thread run "Thread.java" 745]]}

This is breaking my development tooling, far down the dependency hole, but I've traced it back to byte-streams.

jeaye commented 7 years ago

I've updated the issue with a stack trace. To aid in troubleshooting, the issue happens if either of these swaps are present: https://github.com/ztellman/byte-streams/blob/master/src/byte_streams.clj#L123

That is to say, commenting out both allows me to do the :reload-all with no issue.

ztellman commented 7 years ago

This is almost certainly because Clojure blindly re-creates protocols when a namespace is reloaded, which causes incompatible (but identical) versions. I should be able to fix it by doing a check for whether the protocol already exists. Thank you for the report, and sorry to make you track it down. I'll update once a fix is available.

jeaye commented 7 years ago

Thanks for the quick response; it looks like this is the same issue: https://github.com/overtone/overtone/issues/70

They fixed it by wrapping it in a defonce: https://github.com/overtone/overtone/commit/2185920254976e441892485936ac7d1c65450f15

jeaye commented 7 years ago

@ztellman I've currently worked around this by requiring byte-streams in a defonce, which basically is the same as wrapping every source file in one. That's not an ideal solution, but it means that I can actually reload my repl in-place.

The PR I have doesn't seem to resolve all of the issues, but it tackles some. Are you interested in resolving this? If not, will you please document the README to warn others to require it in a defonce?

jeaye commented 7 years ago

@ztellman To be clear, that PR doesn't fix all issues. I still see some issues with two Type objects not being able to be compared, since each Type apparently represents a different type. It doesn't happen on reload, though, it happens when using byte-streams after reload.

ztellman commented 7 years ago

Sorry, I hit merge too quickly. The best fix for this is to wrap the protocol definition, not the conversions themselves. This approach won't fix any other conversions people have defined in their own code.

I've reverted the merge, and am working on a commit that uses this approach.

jeaye commented 7 years ago

@ztellman So, my branch/PR fixes the issue of reloading, but it doesn't tackle everything. The best repro steps I have are listed below. Please open up a fresh repl and run each of these, in sequence. Assuming all has been fixed, there should be no errors. I've marked where things currently fail on both your master and my PR branch.

(require 'byte-streams-simple-check)
(require 'pushback-stream-test)
(require 'byte-streams-test)
(require 'clojure.test)
(clojure.test/run-all-tests)
(require 'byte-streams :reload-all) ; This fails on your branch
(clojure.test/run-all-tests) ; This fails on my branch
ztellman commented 7 years ago

So when I run your example code, I get a spurious failure in one test the first time around, and a deadlock in the second. I'm not really quite sure what's going on in there, but I suspect it isn't related to the original issue. Can you confirm that this unblocks your :reload-all workflow?

jeaye commented 7 years ago

Checking now.

jeaye commented 7 years ago

@ztellman It looks like that does the trick! Naturally, your fix is much more keen than my bumbling around; thanks for putting it in.

With that out of the way, before a new release goes out, are you interested in adding AOT and direct linking? I've made a PR.

jeaye commented 7 years ago

I'm closing this; please let me know when you deploy a new release!