Always take the EventSource lock when manipulating resource notifiers (…

…#738)
toitlang · May 17, 2022 · cd1e3d0 · cd1e3d0
1 parent ae14676
commit cd1e3d0
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 27 deletions.
diff --git a/src/primitive_events.cc b/src/primitive_events.cc
@@ -31,25 +31,10 @@ PRIMITIVE(read_state) {
 }
 
 PRIMITIVE(register_object_notifier) {
-  ARGS(Object, object, ResourceGroup, resource_group, Resource, resource);
+  ARGS(Object, monitor, ResourceGroup, group, Resource, resource);
 
-  ObjectNotifier* notifier = resource->object_notifier();
-  if (notifier) {
-    notifier->update_object(object);
-    return process->program()->null_object();
-  }
-
-  notifier = _new ObjectNotifier(process, object);
-  if (notifier == null) MALLOC_FAILED;
-
-  ObjectNotifyMessage* message = _new ObjectNotifyMessage(notifier);
-  if (message == null) {
-    delete notifier;
-    MALLOC_FAILED;
-  }
-  notifier->set_message(message);
-
-  resource_group->event_source()->set_object_notifier(resource, notifier);
+  EventSource* source = group->event_source();
+  if (!source->update_resource_monitor(resource, process, monitor)) MALLOC_FAILED;
   return process->program()->null_object();
 }
 
@@ -58,8 +43,8 @@ PRIMITIVE(unregister_object_notifier) {
 
   ResourceGroup* group = group_proxy->as_external<ResourceGroup>();
   Resource* resource = resource_proxy->as_external<Resource>();
-  if (group && resource && resource->object_notifier()) {
-    group->event_source()->set_object_notifier(resource, null);
+  if (group && resource) {
+    group->event_source()->delete_resource_monitor(resource);
   }
   return process->program()->null_object();
 }

diff --git a/src/resource.cc b/src/resource.cc
@@ -162,17 +162,37 @@ void EventSource::try_notify(Resource* r, const Locker& locker, bool force) {
   }
 }
 
-void EventSource::set_object_notifier(Resource* r, ObjectNotifier* notifier) {
+bool EventSource::update_resource_monitor(Resource* r, Process* process, Object* monitor) {
   Locker locker(_mutex);
 
+  ObjectNotifier* notifier = r->object_notifier();
   if (notifier) {
-    ASSERT(r->object_notifier() == null);
-    r->set_object_notifier(notifier);
-    if (r->state() != 0) notifier->notify();
+    notifier->update_object(monitor);
   } else {
-    delete r->object_notifier();
-    r->set_object_notifier(null);
+    notifier = _new ObjectNotifier(process, monitor);
+    if (notifier == null) return false;
+
+    ObjectNotifyMessage* message = _new ObjectNotifyMessage(notifier);
+    if (message == null) {
+      delete notifier;
+      return false;
+    }
+    notifier->set_message(message);
+    r->set_object_notifier(notifier);
+  }
+
+  if (r->state() != 0) {
+    notifier->notify();
   }
+  return true;
+}
+
+void EventSource::delete_resource_monitor(Resource* r) {
+  Locker locker(_mutex);
+  ObjectNotifier* notifier = r->object_notifier();
+  if (!notifier) return;
+  delete notifier;
+  r->set_object_notifier(null);
 }
 
 uint32_t EventSource::read_state(Resource* r) {

diff --git a/src/resource.h b/src/resource.h
@@ -226,7 +226,8 @@ class EventSource : public EventSourceList::Element {
   void register_resource_group(ResourceGroup* resource_group);
   virtual void unregister_resource_group(ResourceGroup* resource_group);
 
-  void set_object_notifier(Resource* r, ObjectNotifier* notifier);
+  bool update_resource_monitor(Resource* r, Process* process, Object* monitor);
+  void delete_resource_monitor(Resource* r);
 
   uint32_t read_state(Resource* r);