[NaCl SDK} Add EventListener and EventEmitter to support epoll.

native_client_sdk/src/libraries/nacl_io/event_listener.h

Index: native_client_sdk/src/libraries/nacl_io/event_listener.h
diff --git a/native_client_sdk/src/libraries/nacl_io/event_listener.h b/native_client_sdk/src/libraries/nacl_io/event_listener.h
new file mode 100644
index 0000000000000000000000000000000000000000..b9ee2f81866c0803e24d094817f94f9c8b702cca
--- /dev/null
+++ b/native_client_sdk/src/libraries/nacl_io/event_listener.h
@@ -0,0 +1,140 @@
+/* Copyright (c) 2013 The Chromium Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */

[Sam Clegg, 2013/07/17 17:34:56]

C++ comments

+
+#ifndef LIBRARIES_NACL_IO_EVENT_LISTENER_H_
+#define LIBRARIES_NACL_IO_EVENT_LISTENER_H_
+
+#include <pthread.h>
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "nacl_io/error.h"
+#include "nacl_io/event_emitter.h"
+
+#include "sdk_util/scoped_ref.h"
+
+// Kernel Events
+//
+//   Certain file objects such as pipes or sockets can become signaled when

[binji, 2013/07/17 22:23:20]

why indented?

[noelallen1, 2013/07/18 22:18:16]

Done.

+// read or write buffers become available, or when the connection is torn
+// down.  EventListener provides a mechanism for a thread to wait on
+// specific events from these objects which are derived from  EventEmitters.

[Sam Clegg, 2013/07/17 17:34:56]

Extra space after "derived from".

[noelallen1, 2013/07/17 21:00:18]

Done.

+//
+// EventEmitter and EventListener together provide support for an "epoll"
+// like interface.  See:
+//     http://man7.org/linux/man-pages/man7/epoll.7.html
+//
+//  Behavior of EventListeners
+//    FDs are automatically removed when closed.
+//    KE_SHUTDOWN can not be masked.
+//    KE_SHUTDOWN is only seen if the hangup happens after Wait starts.
+//    Dup'd FDs get their own event info which must also get signaled.
+//    Adding a non streaming FD will fail.
+//    EventEmitters can also be waited on.
+//    It is illegal for an a EventListener to add itself.
+//
+//  Locking
+//    (E) Denotes a lock in the Emitter Object (MountNode)
+//    (L) Denotes a lock in the Listener Object
+//    ?xx? Denotes the lock may or may not be held.
+//
+//    RegisterInfo Object (Track/Update/Free)

[binji, 2013/07/17 22:23:20]

I'm having a hard time following this because the names are not explicit. I know
it's more verbose, but maybe:

EventListener::{Track, Update, Free}
  AUTO_LOCK(EventListener::info_lock_)
    EventEmitter::RegisterEventInfo
      AUTO_LOCK(EventEmitter::emitter_lock_)

+//       Ref Emitter

[binji, 2013/07/17 22:23:20]

Looking at the code below, where does EventListener::Track reference the
emitter? It is given a const reference, so it is just sharing the reference with
the caller.

[noelallen1, 2013/07/18 22:18:16]

We count on the caller having referenced it, but that's already obvious since we
pass in a const ScopedRef& so I'll just take that comment out

+//         (L)info_lock_->(E)emitter_lock_
+//       Deref Emitter
+//
+//    AbandonInfo

[binji, 2013/07/17 22:23:20]

called AbandonedEventInfo below

[noelallen1, 2013/07/18 22:18:16]

Done.

+//       (L)info_lock_
+//
+//    RaiseEvent -> Signal
+//       ?(E)node_lock_?->(E)emitter_lock_->(L)signal_lock_
+//
+//    Wait
+//       (L)info_lock_
+//       (L)signal_lock_
+//
+
+class EventEmitter;
+class EventListener;

[binji, 2013/07/17 22:23:20]

remove

+
+enum KernelEventTypes {
+  KE_READ_READY = 1,
+  KE_WRITE_READY = 2,
+  KE_SHUTDOWN = 4

[Sam Clegg, 2013/07/17 17:34:56]

Is our convention for name enums in the plural like this?

[noelallen1, 2013/07/17 21:00:18]

Done.

+};
+
+struct EventData {
+  // Bit Mask of signaled KernelEvents
+  uint32_t events;
+  uint64_t data;

[Sam Clegg, 2013/07/17 17:34:56]

Maybe user_data?

[noelallen1, 2013/07/17 21:00:18]

Done.

+};
+
+
+// EventListener
+//
+// The EventListener class provides an object to wait on for specific events
+// from EventEmitter objects.

[binji, 2013/07/17 22:23:20]

document why EventListener is also an EventEmitter

[noelallen1, 2013/07/18 22:18:16]

Done.

+class EventListener : public EventEmitter {
+  // Mapping from an id to an EventInfo

[Sam Clegg, 2013/07/17 17:34:56]

Remove comment?

[noelallen1, 2013/07/17 21:00:18]

Done.

+ public:
+   EventListener();
+   ~EventListener();
+
+ protected:
+  // Called prior to free to unregister all EventInfos from the EventEmitters.
+  void Destroy();
+
+ public:
+   // Declated in EventEmitter

[binji, 2013/07/17 22:23:20]

sp: Declared

[noelallen1, 2013/07/18 22:18:16]

Done.

+   virtual uint32_t GetEventStatus();
+   virtual int GetType();
+
+   // Called by EventEmitter to signal the Listener that a new event is
+   // available.
+   void Signal(const ScopedEventInfo& info);
+
+   // Wait for one or more previously defined events to take place.
+   Error Wait(EventData* events, int max, int ms_timeout, int* count);

[Sam Clegg, 2013/07/17 17:34:56]

What are max and count?

Is there a way to wait forever?

[noelallen1, 2013/07/17 21:00:18]

Done.

+
+   // Creates a new EventInfo given the unique |id| and registers it with
+   // the emitter.
+   Error Track(int id,
+               const ScopedRef<EventEmitter>& emitter,

[binji, 2013/07/17 22:23:20]

why not ScopedEventEmitter?

[noelallen1, 2013/07/18 22:18:16]

Done.

+               uint32_t filter,
+               uint64_t data);

[Sam Clegg, 2013/07/17 17:34:56]

user_data?

[noelallen1, 2013/07/17 21:00:18]

Done.

+
+   // Modifies and existing event info referenced by |id|.
+   Error Update(int id, uint32_t filter, uint64_t data);
+
+   // Unregisters the existing event info from the emitter and deletes it.
+   Error Free(int id);
+
+   // Notification that EventEmitter is abandoning the event, do not
+   // unregister or call the emitter after this.
+   void AbandonedEventInfo(const ScopedEventInfo& event);
+
+ private:
+  // Protects the data in the EventInfo map.
+  SimpleLock info_lock_;
+
+  // Map from ID to live a event info.
+  EventInfoMap_t event_info_;

[binji, 2013/07/17 22:23:20]

might be nice to mention "map" in this name somewhere. event_info_map_?
info_map_?

[noelallen1, 2013/07/18 22:18:16]

Done.

+
+  // Protects waiting_, signaled_ and used with the cond_var.

[binji, 2013/07/17 22:23:20]

nit: "used with signal_cond_"

[noelallen1, 2013/07/18 22:18:16]

Done.

+  SimpleLock signal_lock_;
+  pthread_cond_t signal_cond_;
+
+  // The number of threads currently waiting on this Listener.
+  volatile uint32_t waiting_;

[Sam Clegg, 2013/07/17 17:34:56]

Remove volatile?

[noelallen1, 2013/07/17 21:00:18]

Done.

+
+  // Set of event infos signaled during a wait.
+  EventInfoSet_t signaled_;
+};
+
+typedef ScopedRef<EventListener> ScopedEventListener;
+
+#endif  /* LIBRARIES_NACL_IO_EVENT_LISTENER_H_ */

[Sam Clegg, 2013/07/17 17:34:56]

C++ comment