mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Conpherence - kill race conditions around update
Summary: Fixes T6713. The idea is to keep checking what's going on in the update paths that touch the DOM. If we're doing an update or should be doing a different update, then we bail early. This is the type of code + testing that makes me dizzy after awhile, but I think it works... Test Plan: added a "forceStall" parameter to the column view controller, which when specified sleeps for seconds before returning. I then augmented the JS such that the "send message" code for the durable column would specifiy this parameter. For actual testing, I then spammed the heck out of the durable column channel and saw each message only once. I also spammed the column, switched browsers to a user on the same thread in the normal "speedy" view, sent messages there, and also only received one copy Reviewers: chad, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6713 Differential Revision: https://secure.phabricator.com/D12092
This commit is contained in:
parent
c21301d153
commit
98899c822e
3 changed files with 52 additions and 19 deletions
|
@ -352,7 +352,7 @@ return array(
|
||||||
'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761',
|
'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761',
|
||||||
'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18',
|
'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18',
|
||||||
'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de',
|
'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de',
|
||||||
'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'cff1902b',
|
'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'bbc850a4',
|
||||||
'rsrc/js/application/conpherence/behavior-durable-column.js' => 'e975bd12',
|
'rsrc/js/application/conpherence/behavior-durable-column.js' => 'e975bd12',
|
||||||
'rsrc/js/application/conpherence/behavior-menu.js' => 'c4151295',
|
'rsrc/js/application/conpherence/behavior-menu.js' => 'c4151295',
|
||||||
'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861',
|
'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861',
|
||||||
|
@ -517,7 +517,7 @@ return array(
|
||||||
'conpherence-menu-css' => 'c6ac5299',
|
'conpherence-menu-css' => 'c6ac5299',
|
||||||
'conpherence-message-pane-css' => '2526107d',
|
'conpherence-message-pane-css' => '2526107d',
|
||||||
'conpherence-notification-css' => '04a6e10a',
|
'conpherence-notification-css' => '04a6e10a',
|
||||||
'conpherence-thread-manager' => 'cff1902b',
|
'conpherence-thread-manager' => 'bbc850a4',
|
||||||
'conpherence-update-css' => '1099a660',
|
'conpherence-update-css' => '1099a660',
|
||||||
'conpherence-widget-pane-css' => '3d575438',
|
'conpherence-widget-pane-css' => '3d575438',
|
||||||
'differential-changeset-view-css' => '6a8b172a',
|
'differential-changeset-view-css' => '6a8b172a',
|
||||||
|
@ -1684,6 +1684,16 @@ return array(
|
||||||
'javelin-stratcom',
|
'javelin-stratcom',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
),
|
),
|
||||||
|
'bbc850a4' => array(
|
||||||
|
'javelin-dom',
|
||||||
|
'javelin-util',
|
||||||
|
'javelin-stratcom',
|
||||||
|
'javelin-install',
|
||||||
|
'javelin-workflow',
|
||||||
|
'javelin-router',
|
||||||
|
'javelin-behavior-device',
|
||||||
|
'javelin-vector',
|
||||||
|
),
|
||||||
'bbdf75ca' => array(
|
'bbdf75ca' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
|
@ -1764,16 +1774,6 @@ return array(
|
||||||
'javelin-stratcom',
|
'javelin-stratcom',
|
||||||
'phabricator-phtize',
|
'phabricator-phtize',
|
||||||
),
|
),
|
||||||
'cff1902b' => array(
|
|
||||||
'javelin-dom',
|
|
||||||
'javelin-util',
|
|
||||||
'javelin-stratcom',
|
|
||||||
'javelin-install',
|
|
||||||
'javelin-workflow',
|
|
||||||
'javelin-router',
|
|
||||||
'javelin-behavior-device',
|
|
||||||
'javelin-vector',
|
|
||||||
),
|
|
||||||
'd19198c8' => array(
|
'd19198c8' => array(
|
||||||
'javelin-install',
|
'javelin-install',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
|
|
|
@ -58,6 +58,9 @@ final class ConpherenceViewController extends
|
||||||
$content['title'] = $title;
|
$content['title'] = $title;
|
||||||
|
|
||||||
if ($request->isAjax()) {
|
if ($request->isAjax()) {
|
||||||
|
$content['threadID'] = $conpherence->getID();
|
||||||
|
$content['threadPHID'] = $conpherence->getPHID();
|
||||||
|
$content['latestTransactionID'] = $data['latest_transaction_id'];
|
||||||
return id(new AphrontAjaxResponse())->setContent($content);
|
return id(new AphrontAjaxResponse())->setContent($content);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -167,6 +167,23 @@ JX.install('ConpherenceThreadManager', {
|
||||||
}));
|
}));
|
||||||
},
|
},
|
||||||
|
|
||||||
|
_shouldUpdateDOM: function(r) {
|
||||||
|
if (this._updating &&
|
||||||
|
this._updating.threadPHID == this._loadedThreadPHID) {
|
||||||
|
// we have a different, more current update in progress so
|
||||||
|
// return early
|
||||||
|
if (r.latest_transaction_id < this._updating.knownID) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
// we need to let the update code handle things here
|
||||||
|
if (r.latest_transaction_id > this._updating.knownID) {
|
||||||
|
this._updating.knownID = r.latest_transaction_id;
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
},
|
||||||
|
|
||||||
_updateThread: function() {
|
_updateThread: function() {
|
||||||
var params = this._getParams({
|
var params = this._getParams({
|
||||||
action: 'load',
|
action: 'load',
|
||||||
|
@ -177,7 +194,16 @@ JX.install('ConpherenceThreadManager', {
|
||||||
var workflow = new JX.Workflow(uri)
|
var workflow = new JX.Workflow(uri)
|
||||||
.setData(params)
|
.setData(params)
|
||||||
.setHandler(JX.bind(this, function(r) {
|
.setHandler(JX.bind(this, function(r) {
|
||||||
|
if (this._updating &&
|
||||||
|
this._updating.threadPHID == this._loadedThreadPHID) {
|
||||||
|
// we have a different, more current update in progress so
|
||||||
|
// return early
|
||||||
|
if (r.latest_transaction_id < this._updating.knownID) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
this._latestTransactionID = r.latest_transaction_id;
|
this._latestTransactionID = r.latest_transaction_id;
|
||||||
|
this._updating.knownID = r.latest_transaction_id;
|
||||||
this._didUpdateThreadCallback(r);
|
this._didUpdateThreadCallback(r);
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
@ -191,10 +217,10 @@ JX.install('ConpherenceThreadManager', {
|
||||||
};
|
};
|
||||||
workflow.listen(stage, JX.bind(this, function() {
|
workflow.listen(stage, JX.bind(this, function() {
|
||||||
// TODO - do we need to handle if we switch threads somehow?
|
// TODO - do we need to handle if we switch threads somehow?
|
||||||
var need_sync = (this._updating.knownID > this._latestTransactionID);
|
var need_sync = this._updating &&
|
||||||
this._updating = null;
|
(this._updating.knownID > this._latestTransactionID);
|
||||||
if (need_sync) {
|
if (need_sync) {
|
||||||
this._updateThread();
|
return this._updateThread();
|
||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
workflow.start();
|
workflow.start();
|
||||||
|
@ -206,8 +232,10 @@ JX.install('ConpherenceThreadManager', {
|
||||||
var workflow = new JX.Workflow.newFromLink(link)
|
var workflow = new JX.Workflow.newFromLink(link)
|
||||||
.setData(params)
|
.setData(params)
|
||||||
.setHandler(JX.bind(this, function(r) {
|
.setHandler(JX.bind(this, function(r) {
|
||||||
this._latestTransactionID = r.latest_transaction_id;
|
if (this._shouldUpdateDOM(r)) {
|
||||||
this._didUpdateWorkflowCallback(r);
|
this._latestTransactionID = r.latest_transaction_id;
|
||||||
|
this._didUpdateWorkflowCallback(r);
|
||||||
|
}
|
||||||
}));
|
}));
|
||||||
this.syncWorkflow(workflow, params.stage);
|
this.syncWorkflow(workflow, params.stage);
|
||||||
},
|
},
|
||||||
|
@ -249,8 +277,10 @@ JX.install('ConpherenceThreadManager', {
|
||||||
|
|
||||||
var workflow = JX.Workflow.newFromForm(form, params, keep_enabled)
|
var workflow = JX.Workflow.newFromForm(form, params, keep_enabled)
|
||||||
.setHandler(JX.bind(this, function(r) {
|
.setHandler(JX.bind(this, function(r) {
|
||||||
this._latestTransactionID = r.latest_transaction_id;
|
if (this._shouldUpdateDOM(r)) {
|
||||||
this._didSendMessageCallback(r);
|
this._latestTransactionID = r.latest_transaction_id;
|
||||||
|
this._didSendMessageCallback(r);
|
||||||
|
}
|
||||||
}));
|
}));
|
||||||
this.syncWorkflow(workflow, 'finally');
|
this.syncWorkflow(workflow, 'finally');
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue