doobie icon indicating copy to clipboard operation
doobie copied to clipboard

Race conditions in lazy initialization of aliases

Open heksesang opened this issue 5 years ago • 3 comments

There is currently a bug in the lazy initializations of the aliases. What happens is that in our test spec, we have some tests that uses typecheck from AnalysisMatchers. This uses the doobie.hi.HPS alias, which again uses the doobie.hi.FPS alias. Another test runs a query, which leads to the use of the doobie.HPS alias, which also uses the doobie.hi.FPS. Given the right timing, these tests run into a deadlock, for which I have come up with this explanation:

Due to the fact that doobie.package$.HPS$lzycompute does not wait for a lock on doobie.hi.package$, if these tests run on different threads, the thread running the query can call doobie.hi.package$.FPS$lzycompute after the other thread locks doobie.hi.package$ and before it calls doobie.hi.package$.HPS$lzycompute. Then when the thread running analysis matchers try to call doobie.hi.package$.HPS$lzycompute, it gets stuck in static initialization waiting for doobie.hi.package$.FPS$lzycompute to complete on the other thread – but this cannot happen due to the fact that that thread is waiting on a lock on doobie.hi.package$, which cannot get unlocked before doobie.hi.package$.FPS$lzycompute completes.

I think the problem that needs to be fixed is that the initialization of doobie.HPS must refer to doobie.FPS and not doobie.hi.FPS, or else the locking cannot guarantee against deadlocks.

This is extremely fickle due to the timings, so I haven't been able to construct a minimal example yet.

heksesang avatar Feb 15 '21 08:02 heksesang

Like I said on gitter this happened in our work codebase as well, and I introduced a workaround by explicitly loading doobie.HPS and doobie.FC explicitly in some shared test setup.

This means I'm in a position where if we come up with a fix for this I can try it on that project if I disable that workaround. I didn't report the issue at the time because I figured it was infrequent and I didn't have time to minimize it.

And just to clarify: I'm not sure if the observed paths we're the same, but they were at least similar and my analysis at the time was similar to yours

oyvindberg avatar Feb 15 '21 16:02 oyvindberg

I assume the rest of the aliases suffer from the same underlying problem, but whether it manifests itself all depends on how the aliases are used in the different code paths in doobie.

heksesang avatar Feb 15 '21 19:02 heksesang

I'm inclined to remove all lazy vals. I think that should solve the problem? WDYT?

jatcwang avatar Aug 09 '22 22:08 jatcwang