Invalid sync token waits now automatically get released.

Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/9d255f0ca75efea1773fcb718c368a7a6bc4bba4
Cr-Commit-Position: refs/heads/master@{#393409}

[David Yen, 2016-05-12 02:05:04]

Description was changed from

==========
Invalid sync token waits now are ignored completely.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
==========

to

==========
Invalid sync token waits now are ignored completely.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[David Yen, 2016-05-12 02:26:03]

Hmm... the other option is to modify EnsureRelease so it iterates through the
priority queue and removes the callback instead. I was trying to avoid doing
that since it's an NLogN operation but maybe it's better than making all the
callback runs slightly slower with the extra code complexities. What do you
think?

[sunnyps, 2016-05-12 17:41:01]

On 2016/05/12 02:26:03, David Yen wrote:
> Hmm... the other option is to modify EnsureRelease so it iterates through the
> priority queue and removes the callback instead. I was trying to avoid doing
> that since it's an NLogN operation but maybe it's better than making all the
> callback runs slightly slower with the extra code complexities. What do you
> think?

Did you consider using base::CancelableCallback/Closure? Note that those are not
threadsafe though so you need to cancel, bind and run on the same thread. Or
maybe you could take ownership of the release callback (unique_ptr) from the
order fence to avoid refcounting.

[David Yen, 2016-05-12 18:01:41]

On 2016/05/12 17:41:01, sunnyps wrote:
> On 2016/05/12 02:26:03, David Yen wrote:
> > Hmm... the other option is to modify EnsureRelease so it iterates through
the
> > priority queue and removes the callback instead. I was trying to avoid doing
> > that since it's an NLogN operation but maybe it's better than making all the
> > callback runs slightly slower with the extra code complexities. What do you
> > think?
> 
> Did you consider using base::CancelableCallback/Closure? Note that those are
not
> threadsafe though so you need to cancel, bind and run on the same thread. Or
> maybe you could take ownership of the release callback (unique_ptr) from the
> order fence to avoid refcounting.

I implemented iterating the priority queue and popping the bad release callback,
it seems cleaner.

I also added to the test which tests that the wait callback is only called once,
seems like a good test.

[Ken Russell (switch to Gerrit), 2016-05-12 21:29:07]

Description was changed from

==========
Invalid sync token waits now are ignored completely.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Invalid sync token waits now are ignored completely.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[David Yen, 2016-05-12 21:38:08]

Description was changed from

==========
Invalid sync token waits now are ignored completely.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[piman, 2016-05-12 22:03:44]

lgtm

[David Yen, 2016-05-12 22:04:19]

The CQ bit was checked by dyen@chromium.org

[commit-bot: I haz the power, 2016-05-12 22:04:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1975663002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1975663002/60001

[commit-bot: I haz the power, 2016-05-13 00:32:44]

Description was changed from

==========
Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-05-13 00:32:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-13 00:34:42]

Description was changed from

==========
Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Invalid sync token waits now automatically get released.

Previously, invalid sync token waits would automatically release the
fence sync. This would guard against invalid waits which pretended it
would release a fence sync while issuing waits to cause dead locks.

This worked, however it would allow an evil renderer to release fences
on the browser which would invalidate waits. A better solution is to
merely cause the invalid wait to be ignored instead.

R=piman@chromium.org
BUG=606112
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/9d255f0ca75efea1773fcb718c368a7a6bc4bba4
Cr-Commit-Position: refs/heads/master@{#393409}
==========

[commit-bot: I haz the power, 2016-05-13 00:34:43]

Patchset 4 (id:??) landed as
https://crrev.com/9d255f0ca75efea1773fcb718c368a7a6bc4bba4
Cr-Commit-Position: refs/heads/master@{#393409}