Page MenuHomePhabricator

Fix some daemon errors related to multiple/out-of-order events

Authored by epriestley on May 26 2016, 1:13 AM.
Referenced Files
Unknown Object (File)
Feb 25 2023, 2:44 AM
Unknown Object (File)
Feb 22 2023, 3:03 AM
Unknown Object (File)
Feb 20 2023, 8:13 PM
Unknown Object (File)
Feb 15 2023, 7:44 PM
Unknown Object (File)
Feb 5 2023, 5:23 PM
Unknown Object (File)
Feb 5 2023, 8:47 AM
Unknown Object (File)
Jan 20 2023, 11:52 PM
Unknown Object (File)
Jan 19 2023, 5:08 PM



Ref T10811. Currently, we can emit the "willExit" event multiple times, or emit it and then emit a "log" event.

This causes some problems with the Phabricator listener, which cleans up resources the first time it sees this event. When it gets the second copy (or logs afterward) it fails to process events.

Instead, emit it only once we're actually cleaning up the daemon handle. This guarantees it emits last and emits exactly once.

I believe this only cleans up logs, but it's possible it also improves stability.

Test Plan

You can reproduce this issue like this:

  • Run bin/phd debug hangforever.
  • Kill it with ^C^C.
  • Most of the time, it will emit errors.

With --trace, you can see additional information. After this patch, it no longer emits errors but still successfully marks the daemon as exited, and the willExit event is always emitted exactly once and emitted last.

Diff Detail

rPHU libphutil
Lint Not Applicable
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Fix some daemon errors related to multiple/out-of-order events.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

Specifically, the error this fixes looks like this:

[2016-05-25 17:45:35] EXCEPTION: (Exception) No such daemon "39084:6ra7uz"! at [<phabricator>/src/applications/daemon/event/PhabricatorDaemonEventListener.php:120]
arcanist(head=master, ref.master=2234c8cacc21), corgi(head=master, ref.master=cfed644883b0), instances(head=stable, ref.master=47349cf90a41, ref.stable=788b3aa42b2a), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=stable, ref.master=189600e4116e, ref.stable=9f399247b1b1, custom=8), phutil(head=locale1, ref.master=bd56873ae4c0, ref.locale1=092a19c216e9), services(head=master, ref.master=e07ec904b0b9)
  #0 <#2> PhabricatorDaemonEventListener::getDaemon(string) called at [<phabricator>/src/applications/daemon/event/PhabricatorDaemonEventListener.php:63]
  #1 <#2> PhabricatorDaemonEventListener::handleLogEvent(PhutilEvent) called at [<phabricator>/src/applications/daemon/event/PhabricatorDaemonEventListener.php:24]
  #2 <#2> PhabricatorDaemonEventListener::handleEvent(PhutilEvent) called at [<phutil>/src/events/PhutilEventEngine.php:65]
  #3 <#2> PhutilEventEngine::dispatchEvent(PhutilEvent) called at [<phutil>/src/daemon/PhutilDaemonHandle.php:248]
  #4 phlog(Exception) called at [<phutil>/src/daemon/PhutilDaemonHandle.php:250]
  #5 PhutilDaemonHandle::dispatchEvent(string, array) called at [<phutil>/src/daemon/PhutilDaemonHandle.php:418]
  #6 PhutilDaemonHandle::logMessage(string, string) called at [<phutil>/src/daemon/PhutilDaemonHandle.php:117]
  #7 PhutilDaemonHandle::update() called at [<phutil>/src/daemon/PhutilDaemonOverseer.php:209]
  #8 PhutilDaemonOverseer::run() called at [<phabricator>/scripts/daemon/launch_daemon.php:18]

That happened because we'd emit the event once, and throw away 39084:6ra7uz while processing it. Then we'd emit the event again (or emit a log event) and the listener would try to look up 39084:6ra7uz, but fail because we just threw it away.

chad edited edge metadata.
This revision is now accepted and ready to land.May 26 2016, 3:01 PM
This revision was automatically updated to reflect the committed changes.