Watched path fragility in IMBLibraryController

[danielpunkass]

I believe there is a fragility in our watched paths strategy based on the use of a node's file system path as the key for adding/removing watched status on IMBFSEventsWatcher.

The code in IMBLibraryController is conscious of the fact that more than one node may correspond to the same watched path. For example in -[IMBLibraryController _coalescedFSEventsCallback], it iterates over all nodes that correlate to a given changed watched path.

However, the strategy for maintaining an accurate registry of watched paths is flawed in that every node, in the node replacement method: -[IMBLibraryController _replaceNode:withNode:parentNodeIdentifier:], dictates by itself whether its corresponding path should be added to, but more importantly, whether it should be removed from the existing registration of watched file system paths.

This leads to a situation where, for example, if node A and node B are added to the library, and point to the same path:

A -> /my/path
B -> /my/path

Then each of them in their original add to the node list causes "/my/path" to be added to the fsEvents registration. Then, at every reload, in _replaceNode:withNode:parentNodeIdentifier:, the path will be subsequently removed and then re-added the fsEvents registration. So far, no problem.

The issue would arise when, for example, node B is removed, while node A remains uneffected. In this scenario, I believe the _replaceNode process for B would have "/my/path" removed from registratoin, while node A elsewhere in the library hierarchy still expects to be notified of changes to that path.

I believe this is issue is probably at the heart of periodic, vexing reports that libraries or folders just "fail to update" until manually refreshed or the app is quit and restarted.

A simple fix could be to introduce some kind of reference counting in IMBFSEventsWatcher, so that ever addPath call must be balanced by a removePath before the file path is actually removed from registration.

I am not so concerned with the UKK stuff because I am frankly not sure it's even being used any longer with our new 10.6+ focus.

Thoughts?

[danielpunkass]

While examining this issue it occurs to me that there precisely because a watched path can be actively monitored on behalf of different nodes (e.g. two top level nodes happen to have the same path), it's actually not fair to throw an exception in IMBFSEventsWatcher when the same path is attempted to be registered twice. It should be registered and issue callbacks to the delegate. I think this supports the idea of putting reference counting of some kind in IMBFSEventsWatcher itself, so that clients can rest assured that a contract is fulfilled: after N addPaths for a given path, you will receive change notices until N removePath calls have been made.