jackrusher/spicerack

Error when deserializing

plexus opened this issue · 4 comments

plexus commented

It seems like MapDB allows storing things that it is unable to retrieve, thus rendering the database inaccessible.

I'm not 100% sure what I did here, but it would appear I stored an anonymous function.

Would it be possible to detect cases like these so the error happens when storing the value, rather than later on when retrieving it?

org.mapdb.DBException$SerializationError
	at org.mapdb.StoreDirectAbstract.deserialize(StoreDirectAbstract.kt:234)
	at org.mapdb.StoreDirect.get(StoreDirect.kt:546)
	at org.mapdb.HTreeMap.valueUnwrap(HTreeMap.kt:1198)
	at org.mapdb.HTreeMap.getprotected(HTreeMap.kt:644)
	at org.mapdb.HTreeMap.containsKey(HTreeMap.kt:584)
	at clojure.lang.RT.getFrom(RT.java:786)
	at clojure.lang.RT.get(RT.java:778)
	at redolist.core$new_routes$fn__58298$fn__58299.invoke(core.clj:51)
	at compojure.core$make_route$fn__30790.invoke(core.clj:135)
	at compojure.core$wrap_route_middleware$fn__30783.invoke(core.clj:122)
	at compojure.core$wrap_route_info$fn__30787.invoke(core.clj:126)
	at compojure.core$if_route$fn__30735.invoke(core.clj:45)
	at compojure.core$if_method$fn__30725.invoke(core.clj:27)
	at compojure.core$routing$fn__30797.invoke(core.clj:151)
	at clojure.core$some.invokeStatic(core.clj:2674)
	at clojure.core$some.invoke(core.clj:2665)
	at compojure.core$routing.invokeStatic(core.clj:151)
	at compojure.core$routing.doInvoke(core.clj:148)
	at clojure.lang.RestFn.invoke(RestFn.java:460)
	at redolist.core$new_routes$fn__58298.invoke(core.clj:49)
	at compojure.core$if_context$fn__30861.invoke(core.clj:220)
	at compojure.core$routing$fn__30797.invoke(core.clj:151)
	at clojure.core$some.invokeStatic(core.clj:2674)
	at clojure.core$some.invoke(core.clj:2665)
	at compojure.core$routing.invokeStatic(core.clj:151)
	at compojure.core$routing.doInvoke(core.clj:148)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at compojure.core$routes$fn__30801.invoke(core.clj:156)
	at compojure.core$routing$fn__30797.invoke(core.clj:151)
	at clojure.core$some.invokeStatic(core.clj:2674)
	at clojure.core$some.invoke(core.clj:2665)
	at compojure.core$routing.invokeStatic(core.clj:151)
	at compojure.core$routing.doInvoke(core.clj:148)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at compojure.core$routes$fn__30801.invoke(core.clj:156)
	at ring.middleware.keyword_params$wrap_keyword_params$fn__32128.invoke(keyword_params.clj:35)
	at ring.middleware.params$wrap_params$fn__32404.invoke(params.clj:64)
	at ring.middleware.absolute_redirects$wrap_absolute_redirects$fn__32578.invoke(absolute_redirects.clj:38)
	at ring.middleware.content_type$wrap_content_type$fn__32531.invoke(content_type.clj:30)
	at ring.middleware.default_charset$wrap_default_charset$fn__32553.invoke(default_charset.clj:26)
	at ring.middleware.not_modified$wrap_not_modified$fn__32506.invoke(not_modified.clj:52)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_params$wrap_format_params$fn__34361.invoke(format_params.clj:119)
	at ring.middleware.format_response$wrap_format_response$fn__34538.invoke(format_response.clj:194)
	at org.httpkit.server.HttpHandler.run(RingHandler.java:91)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.io.IOException: java.lang.ClassNotFoundException: redolist.core$new_routes$fn__58298$fn__58306$fn__58308$fn__58309
	at org.mapdb.serializer.SerializerJava.deserialize(SerializerJava.java:30)
	at org.mapdb.StoreDirectAbstract.deserialize(StoreDirectAbstract.kt:229)
	... 57 more
Caused by: java.lang.ClassNotFoundException: redolist.core$new_routes$fn__58298$fn__58306$fn__58308$fn__58309
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:348)
	at java.io.ObjectInputStream.resolveClass(ObjectInputStream.java:628)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1620)
	at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1521)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1781)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1353)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2018)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:1942)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1808)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1353)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2018)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:1942)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1808)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1353)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:373)
	at org.mapdb.serializer.SerializerJava.deserialize(SerializerJava.java:28)
	... 58 more
jackrusher commented

Would it be possible to provide a failing test case?

plexus commented

I dug into this a bit more. Here's an example failing test case

(deftest storing-bad-values
  (testing "fail early when storing functions"
    (cleanup)
    (with-open [db (open-database test-filename)]
      (let [hm (open-hashmap db "test-hashmap")]
        (is (thrown? Throwable (do (put! hm :lambda (fn []))
                                   ;; it's the "get" that triggers an error
                                   ;; (get hm :lambda)
                                   )))))))

The problem is that Clojure functions are Serializable (I found this somewhat surprising, but it turns out to be true), and so MapDB stores them as a Java Object. When it pulls them out and tries to deserialize them, it does its own class lookup on the function "class", something like "spicerack.core_test$eval20554$fn__20555", which fails.

I'm happy to call this "garbage-in-garbage-out" and close the issue. It's just unfortunate that the problem is only triggered when reading, thus rendering your database (at least partially) unusable.

A possible "solution" could be

(defn put!
  "Write the key/value pair (`k`,`v`) to hashmap `m`. Returns v."
  [m k v]
  (assert (not (instance? clojure.lang.AFunction v)) "The stored value should not be a function.")
  (.put m k v)
  v)

(You have to check for AFunction here, IFn or AFn include things like keywords and maps.)

For the curious: yes Clojure functions are actually serializable. No idea how will this works across reboots or across runtimes though 😄

((let [serialized
       (let [baos (java.io.ByteArrayOutputStream.)
             oos (java.io.ObjectOutputStream. baos)]
         (.writeObject oos (fn [] :sentinel))
         (.toByteArray baos))]

   (let [bais (java.io.ByteArrayInputStream. serialized)
         ois (java.io.ObjectInputStream. bais)]
     (.readObject ois))))
;;=> :sentinel
jackrusher commented

Thanks for the more detailed bug report!

The Clojure compiler generates Java classes that it then pushes out to class files as it goes, even for anonymous functions. My guess is that they are Serializable to facilitate that process. Sadly, that doesn't help us here, because when they are re-loaded from the DB they are orphaned from the other stuff that would be in those class files. 😿

A workaround for cases where this functionality is desired is to use serializable-fn for those functions.

I agree with you that it's unfortunate that someone can put rubbish in their database this way, but at the same time I'm not willing to add the overhead of a Java reflection call to every put. We should document this case instead.

plexus commented

That makes sense.