New issue
Advanced search Search tips

Issue 599494 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2016
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

RemoteSecurityKeyMessageReaderTest.MultipleMessages failure on Dr. Memory bot

Project Member Reported by joedow@chromium.org, Mar 31 2016

Issue description

RemoteSecurityKeyMessageReaderTest.MultipleMessages:
[3144:216:0330/224907:1854773:ERROR:remote_security_key_message_reader.cc(94)] Failed to read message: 1
e:\b\build\slave\drm-cr-64\build\src\remoting\host\security_key\remote_security_key_message_reader_unittest.cc(200): error: Value of: messages_received_.size()
Actual: 1
Expected: payloads.size()
Which is: 11
 

Comment 1 by joedow@chromium.org, Mar 31 2016

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0c1837abcd55db74ad07220bcc13e4c7dfb7e82

commit d0c1837abcd55db74ad07220bcc13e4c7dfb7e82
Author: joedow <joedow@chromium.org>
Date: Thu Mar 31 19:09:51 2016

Fixing a Dr. memory failure in the remoting unit tests.

The test I checked in yesterday had a race condition in it which was detected
by the Dr. Memory bot (they run more slowly than the normal bots).  On a
normal bot, my runloop quits when the first (of ~11) message is written but
the other messages are processed and added to the vector by the time it is
checked.  On the Dr. Memory bot, this condition was exposed and the size check
failed.  The fix is to ensure that we are processing each message as it is
written/received and validating the size afterwards.

BUG= 599494 

Review URL: https://codereview.chromium.org/1844423002

Cr-Commit-Position: refs/heads/master@{#384354}

[modify] https://crrev.com/d0c1837abcd55db74ad07220bcc13e4c7dfb7e82/remoting/host/security_key/remote_security_key_message_reader_unittest.cc

Comment 3 by joedow@chromium.org, Mar 31 2016

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/512b6ea08ab2eea17f2af00b0cf988ce1a4afd2d

commit 512b6ea08ab2eea17f2af00b0cf988ce1a4afd2d
Author: benwells <benwells@chromium.org>
Date: Fri Apr 01 04:52:09 2016

Revert of Fixing a Dr. memory failure in the remoting unit tests. (patchset #3 id:40001 of https://codereview.chromium.org/1844423002/ )

Reason for revert:
However, this was the data posted to the server:

revert_reason: This change has caused failures on TSAN, and also caused the DrMemory bot to timeout.

See https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/18731

and

https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/9978
revert_cq: 1
revert_reason_textarea: This change has caused failures on TSAN, and also caused the DrMemory bot to timeout.

See https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/18731

and

https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/9978
xsrf_token: 3f3abe97db5300c6623390914f94b515

Original issue's description:
> Fixing a Dr. memory failure in the remoting unit tests.
>
> The test I checked in yesterday had a race condition in it which was detected
> by the Dr. Memory bot (they run more slowly than the normal bots).  On a
> normal bot, my runloop quits when the first (of ~11) message is written but
> the other messages are processed and added to the vector by the time it is
> checked.  On the Dr. Memory bot, this condition was exposed and the size check
> failed.  The fix is to ensure that we are processing each message as it is
> written/received and validating the size afterwards.
>
> BUG= 599494 
>
> Committed: https://crrev.com/d0c1837abcd55db74ad07220bcc13e4c7dfb7e82
> Cr-Commit-Position: refs/heads/master@{#384354}

TBR=sergeyu@chromium.org,joedow@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 599494 

Review URL: https://codereview.chromium.org/1849023004

Cr-Commit-Position: refs/heads/master@{#384486}

[modify] https://crrev.com/512b6ea08ab2eea17f2af00b0cf988ce1a4afd2d/remoting/host/security_key/remote_security_key_message_reader_unittest.cc

Status: Started (was: Fixed)
Updating bug since the original fix was reverted.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c705be10c12997d771afcd7c0eb301a311adfb8a

commit c705be10c12997d771afcd7c0eb301a311adfb8a
Author: joedow <joedow@chromium.org>
Date: Tue Apr 05 02:55:25 2016

Fixing a Dr. memory failure in the remoting unit tests.

The test I checked in yesterday had a race condition in it which was detected
by the Dr. Memory bot (they run more slowly than the normal bots).  On a
normal bot, my runloop quits when the first (of ~11) message is written but
the other messages are processed and added to the vector by the time it is
checked.  On the Dr. Memory bot, this condition was exposed and the size check
failed.  The fix is to ensure that we are processing each message as it is
written/received and validating the size afterwards.

This is a reland of: https://codereview.chromium.org/1844423002/
Due to changes to the base files, I am sending this out as a new patch instead
of using the old CL.

BUG= 599494 

Review URL: https://codereview.chromium.org/1860693003

Cr-Commit-Position: refs/heads/master@{#385093}

[modify] https://crrev.com/c705be10c12997d771afcd7c0eb301a311adfb8a/remoting/host/security_key/remote_security_key_message_reader_impl.cc
[modify] https://crrev.com/c705be10c12997d771afcd7c0eb301a311adfb8a/remoting/host/security_key/remote_security_key_message_reader_impl_unittest.cc

Status: Verified (was: Started)

Sign in to add a comment