dissoc injected dependencies at shutdown

[hlship]

Given that the system will "inject" dependencies into a component map (via assoc), I believe it should, at system shutdown, dissoc those same keys.

In my particular situation, I had a component that was a wrapper around a C3P0 connection pool. As part of stop, the connection pool was itself .close-d.

However, the component had been propagated to another component as a dependency.

After the successful shutdown, the SystemMap is printed to the console (in the REPL). The migrations component still included the injected key to the connection pool manager, and so the C3P0 connection pool received toString(), which (and this is a separate comment on the quality of C3P0) caused it to spin its wheels for 30 seconds and eventually report an OutOfMemoryException wrapped in an InvocationTargetException.

I've modified my code to dissoc any dependencies injected by the system, but I strongly feel that this needs to be either strongly noted in the documentation OR automatically performed by stop. I'd prefer the latter.

And I'd be happy to submit a patch, but I know how you feel about them.

[stuartsierra]

Thanks for the experience report. I'll have to think about this one for a while. I don't want to preclude other lifecycles, and the dependency propagation is tricky. There may be a problem with update-system-reverse leaving stale versions of dependencies in the system, but I need to spend some more time to figure it out.

One thing: components are often records. (dissoc r k) on a record r returns a plain map if k is one of the record's defined fields so, to avoid losing type information, the only safe action would be (assoc r k nil).

[stuartsierra]

After some work and experimentation on the reverse-dependencies branch, I don't think this is going to work. Propagating dependencies backwards proved very difficult to do correctly. Records-as-components means that dissoc is not safe, and assoc ... nil doesn't guarantee the component is in the same state it was before start.

I never intended that stop should be the complete inverse of start. As I noted on #11, it may be desirable to retain state so that the system can be re-started.

I also never intended that system maps should be printable. They will always have lots of repetition, and may contain large objects such as caches.

I will make a note of this in the README, probably similar to 03c263c.

[stuartsierra]

Behavior of stop on systems is documented in the README as of dafe965

[stuartsierra]

As of release 0.2.2, system map defines a print-method to avoid accidentally printing large objects at the REPL.

[hlship]

I noticed and I like it, makes a big improvement.

On Thursday, September 18, 2014, Stuart Sierra [email protected]
wrote:

As of release 0.2.2
https://github.com/stuartsierra/component/blob/component-0.2.2/README.md#change-log,
system map defines a print-method to avoid accidentally printing large
objects at the REPL.

—
Reply to this email directly or view it on GitHub
#9 (comment)
.

Howard M. Lewis Ship

Creator of Apache Tapestry

The source for Tapestry training, mentoring and support. Contact me to
learn how I can get you up and productive in Tapestry fast!

(971) 678-5210
http://howardlewisship.com
@hlship