WebRTC Code Reviews
Help | Chromium Project | Sign in
(10886)

Issue 38829005: Remove POSIX channel_transport ConditionVariables. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 9 months ago by pbos
Modified:
3 years, 10 months ago
Reviewers:
tommi
CC:
webrtc-reviews_webrtc.org, tterriberry
Base URL:
http://webrtc.googlecode.com/svn/trunk
Project:
webrtc
Visibility:
Public.

Description

Remove POSIX channel_transport ConditionVariables. ConditionVariable is barely used so I'm working on getting rid of it. The implementation of it is complex and seems to have issues. BUG= R=tommi@webrtc.org

Patch Set 1 #

Total comments: 2

Messages

Total messages: 2 (0 generated)
pbos
Hey tommi@, hope you agree that we should get rid of ConditionVariable since it's virtually ...
4 years, 9 months ago (2015-02-05 15:45:49 UTC) #1
tommi
4 years, 9 months ago (2015-02-05 16:04:50 UTC) #2
I agree with you on condition variable.  Maybe we can revisit that later when we
have full access to STL and don't have to support XP.  A coworker just mentioned
to me that the our XP implementation has race issues, so good to get rid of.

https://review.webrtc.org/38829005/diff/1/webrtc/test/channel_transport/udp_s...
File webrtc/test/channel_transport/udp_socket_posix.cc (right):

https://review.webrtc.org/38829005/diff/1/webrtc/test/channel_transport/udp_s...
webrtc/test/channel_transport/udp_socket_posix.cc:261:
_closeBlockingCompletedEvent->Wait(WEBRTC_EVENT_INFINITE);
with these changes, if the Wait() is satisfied but the associated bool is not
set, wouldn't that be a serious error?  maybe I'm misunderstanding how it's
supposed to work, but how does the loop compare to e.g. this:

if (!_closeBlockingCompleted) {
   _closeBlockingCompletedEvent->Wait(WEBRTC_EVENT_INFINITE);
  DCHECK(_closeBlockingCompleted);
}

https://review.webrtc.org/38829005/diff/1/webrtc/test/channel_transport/udp_s...
File webrtc/test/channel_transport/udp_socket_posix.h (right):

https://review.webrtc.org/38829005/diff/1/webrtc/test/channel_transport/udp_s...
webrtc/test/channel_transport/udp_socket_posix.h:79: EventWrapper*
_closeBlockingCompletedEvent;
can you change these to scoped_ptr's?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted