File modification events are getting lost in PollingWatchService

[mechkg]

While investigating sbt/sbt#3775 I think I have narrowed the bug down to the interaction between PollingWatchService.pollEvents and PollingWatchService.PollingThread.addEvent.

Events often get lost without being properly consumed (#82 is probably related).

UPDATE: the conclusion below is wrong, see comments further down.

As far as I understand the only reason that could happen is if some events get added between these two statements:

io/io/src/main/scala/sbt/io/PollingWatchService.scala

Lines 61 to 64 in e8c9757

	val events = thread.keysWithEvents.map { k =>
	k -> k.pollEvents().asScala.asInstanceOf[Seq[WatchEvent[JPath]]]
	}
	thread.keysWithEvents.clear()

so thread.keysWithEvents.clear deletes events that weren't consumed. This should be prevented by the synchronized guard.

It is difficult to set up my own modified sbt build to properly test this but at a glance I find this line suspicious:

io/io/src/main/scala/sbt/io/PollingWatchService.scala

Line 105 in e8c9757

keysWithEvents.notifyAll()

I don't understand the purpose of the notifyAll call at this point and I suspect that it could break the intended synchronization behaviour.

[dwijnand]

Thank you for the insight @mechkg, I'll look into what you've found (it works well with sbt/sbt#3687, sbt/sbt#3695, #82, and sbt/sbt#3775 which I'm look at).

[mechkg]

In PollingWatchService.pollEvents events are gathered as follows:

   val events = thread.keysWithEvents.map { k =>
        k -> k.pollEvents().asScala.asInstanceOf[Seq[WatchEvent[JPath]]]
      }

PollingWatchKey.pollEvents in turn does this:

  origin.keysWithEvents -= this

So keysWithEvents is being modified by PollingWatchKey.pollEvents at the same time as it's being mapped over in PollingWatchService.pollEvents which looks very dangerous. Usually in Java this would result in a ConcurrentModificationException but it does not in this case, I wonder why?

[mechkg]

Simply removing this line:

 origin.keysWithEvents -= this

apparently makes the problem go away. It looks like it is not necessary because keysWithEvents is getting cleared at the end of PollingWatchService.pollEvents anyway, but I am not sure if any other code depends on this behaviour?

[dwijnand]

Fixed with #90.