Fix AsyncIterator race condition when first element in source raises

[clintval]

This PR fixes a race condition with the exception-raising mechanics of AsyncIterator.

The bug occurs when the head (or the first few) elements of the source iterator raise exceptions. The prior mechanics of AsyncIterator might run the check-and-raise call in the hasNext() block before any of the first elements are actually computed. This means that exceptions can be swallowed and your iterator will be silently truncated to 0-length.

A quick example of how the race condition will create missing data:

 def raiseOnTen(num: Int): Int = if (num == 10) throw new IllegalArgumentException else num
 new AsyncIterator(Seq(10, 11, 12, 13).iterator.map(raiseOnTen)).start().toSeq == Seq.empty

[clintval]

Tests had passed on my local... Looks like there are some more race conditions....

[nh13]

@clintval let me know if you want me to take a look at the code, the additional race condition, or otherwise

[clintval]

If you're volunteering, that would kind! It's not easy code to debug.

I believe I fixed 1 race condition but I am certain there are also others. I might do some OSS contributions over break and could get to this. I actually think I know how to fix this one but no promises.

[nh13]

Why don't you give it a shot, then I will.

[clintval]

@nh13 I'm certain I improved the implementation. I think I fixed up to 2 ways the race condition can manifest. But unfortunately I do not know how to prove there are no more race conditions. The race condition occurs when there are exceptions raised within the input source iterator which will short-circuit the iterator and (with the prior implementation) go silent.

[codecov-commenter]

Codecov Report

Merging #74 (79f06c7) into master (b851fd3) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   93.23%   93.25%   +0.02%     
==========================================
  Files          27       27              
  Lines         857      860       +3     
  Branches       57       75      +18     
==========================================
+ Hits          799      802       +3     
  Misses         58       58              

Flag	Coverage Δ
unittests	93.25% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../fulcrumgenomics/commons/async/AsyncIterator.scala	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b851fd3...79f06c7. Read the comment docs.

[clintval]

If you remove my fix in the class and run the new test you will see it often fails:

❯ testOnly com.fulcrumgenomics.commons.async.AsyncIteratorTest
[info] - should correctly propagate an exception that originates from within the source iterator *** FAILED ***
[info]   Expected exception java.lang.IllegalArgumentException to be thrown, but no exception was thrown (AsyncIteratorTest.scala:51)

[clintval]

I thought about it for a few more hours and realized I made the race condition occur more infrequently but I did not actually solve it. Now I believe I have solved it (famous last words).

[nh13]

@clintval thank-you thank-you!!!