I noticed osquery_events_tests_inotifytests-test is a flaky test because of getEventSubscriber:

  if (!exists(name_id)) {
    LOG(ERROR) << "Requested unknown event subscriber: " + name_id;
    return nullptr;
  }
  return ef.event_subs_.at(name_id);

This is an unprotected race condition and one (of few) callsite is within EventPublisher::fire. This test exercises the race because it may remove the specific event subscriber before all inotify events have been serviced. Thus the exists works and the at throws.

There are two options IMO to address the issue:

• Add a lock (which means an embedded ReadLock within a same-thread's WriteLock)
• Refactor to include a getEventSubscriberUnsafe that does not lock and keep one that waterfalls, which does.

The first option requires a RecursiveMutex since additional callsites are requesting a WriteLock then needing a ReadLock. This seems like the simplest approach.

There is no performance impact measured at the millisecond granularity incurred by introducing the lock and moving to a RecursiveMutex. Note that this code change highlights a few places where the code could be improved, specifically not searching a map twice. I do not want to perform these optimizations within the same PR.