search

ubermichael / isetools

Tools for parsing data for the Internet Shakespeare Editions
GNU General Public License v2.0
2 stars 3 forks source link

Log doesn't play well with multi-threading #42

Closed telic closed 2 years ago

telic commented 8 years ago

Since we're now using the isetools as a library for a multi-threaded web service, having a singleton Log really doesn't make sense. Separate users validating separate documents don't want to see each other's error messages :) Worse than that, because Log is not synchronized, multi-threaded use may even lead to crashes...

It looks like @emmental made some progress on rewriting Log so that there was a separate instance for each DOM, but I'm not sure how far he got with that approach. It looks like he has a branch for it at https://github.com/emmental/isetools/tree/separate_logs.

An alternate strategy might be simply to make Log a singleton-per-thread using ThreadLocals. I imagine that would require a lot less work.

emmental commented 8 years ago

I haven't integrated the log into the deployed tools yet (it's not in 'allchanges'), but it is "finished" in that it appears to work and all tests which use the log pass.

ubermichael commented 6 years ago

Interesting implementation. You've kinda moved the singleton inside DOM which makes me a little bit uneasy.

In reviewing it, I came across this line

https://github.com/emmental/isetools/blob/2a4d05af420ea4110dc6cd3214eef7c364056012/src/main/java/ca/nines/ise/dom/DOM.java#L103

That copy constructor assigns the log to the new DOM as it, a very shallow copy. So the new and old DOMs share the same log. That seems counter to your goals.