New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 1572 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Apr 2018
Cc:
Components:
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

Windows bug in webrtc/system_wrappers/interface/tick_util.h

Reported by tim...@gmail.com, Apr 3 2013

Issue description

What steps will reproduce the problem?

In tick_util.h has following code;

 (C1) DWORD now = timeGetTime();                                      
 (C2) DWORD old = InterlockedExchange(lastTimeGetTimePtr, now);
 (C3) if ((now < old) && (old > 0xf0000000 && now < 0x0fffffff)) { numWrapTimeGetTime++; }
 (C4) result = now + (numWrapTimeGetTime<<32);

 Thread Sequence (now/old)

 Thread 1     Thread 2      Thread 3       Last       Wrap
 (C1) 99/                                   90         0
 Context Switch
              (C1) 2/
              (C2) 2/90                      2         
              (C3) Yes                                 1
              (C4) result:102
 (C2) 99/2                                  99
 (C3) no
 (C4) result:199 (BUG!)
                            (C1) 5/
                            (C2) 5/99        5
                            (C3) Yes                   2
                            (C4) result:205 (BUG!)

What is the expected result?

  Correct absolute timestamp returned

What do you see instead?

  Incorrect timestamp

What version of the product are you using? On what operating system?
 
  Windows
 
Project Member

Comment 1 by braveyao@webrtc.org, Apr 8 2013

Cc: braveyao@webrtc.org
Owner: pwestin@webrtc.org
@Patrick, could you please help to take a look at this?
Project Member

Comment 2 by pwestin@webrtc.org, Apr 9 2013

Owner: henrike@webrtc.org
Status: Assigned

Comment 3 by henrike@webrtc.org, Apr 11 2013

Cc: henrike@webrtc.org wu@webrtc.org
Labels: -Pri-2 Pri-3
Owner: niklas.enbom@webrtc.org
I agree with the assessment that this code is not thread safe during wraparound.

The simple way of fixing this would be to acquire a lock but doing so would likely cause a performance hit so we don't want to do that.

Trying to fix this in a lock free way is risky as it is very likely to introduce subtle issues.

The likelihood of this happening is very small. On average a wraparound happens every ~49.7/2 days. In addition, at the time of wraparound a thread that was about to increment the wraparound counter must have been preempted by another thread beating it to the punch for there to be a data race.

If a very incorrect timestamp is generated e.g. to mark the capture time of a frame it would be caught by sanity checks. I.e. the worst thing that would happen is that a frame would be dropped.

The risk vs reward of trying to fix this is heavily skewed towards risk. Assigning to Niklas to figure out if this is something we want to consider in the very long term.

Comment 4 by vrk@webrtc.org, Dec 17 2014

Labels: Area-Internals

Comment 5 Deleted

Project Member

Comment 6 by kjellander@webrtc.org, Dec 1 2016

Cc: -henrike@webrtc.org
Project Member

Comment 7 by anatolid@webrtc.org, Jan 11 2017

Cc: niklas.enbom@webrtc.org
Owner: ----
Status: Untriaged (was: Assigned)
Moving back to triage since no meaningful updates have happened since more than a year ago. Is this still a relevant issue?
Project Member

Comment 8 by anatolid@chromium.org, Sep 27 2017

Ping for triaging.
Project Member

Comment 9 by anatolid@webrtc.org, Apr 26 2018

Ping.
Project Member

Comment 10 by kwiberg@webrtc.org, Apr 27 2018

Cc: sprang@webrtc.org nisse@webrtc.org
Status: Fixed (was: Untriaged)
This code was removed in January 2016 [https://codereview.webrtc.org/1639543005], and the entire file was removed in May 2016 [https://codereview.webrtc.org/1888593004].

Sign in to add a comment