New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #1354 #1356

Merged
merged 1 commit into from Dec 18, 2015

Conversation

Projects
None yet
2 participants
@9il
Contributor

9il commented Dec 17, 2015

No description provided.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 18, 2015

Member

Wouldn't it be sufficient to move the runTask below the initialization of those synchronization primitives?

Member

s-ludwig commented Dec 18, 2015

Wouldn't it be sufficient to move the runTask below the initialization of those synchronization primitives?

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Dec 18, 2015

Contributor

It would be sufficient exactly for this bug. However, there is another bug: without the lock a socket can be closed before initialization of m_reader variable and close method would access uninitialized m_reader.

Contributor

9il commented Dec 18, 2015

It would be sufficient exactly for this bug. However, there is another bug: without the lock a socket can be closed before initialization of m_reader variable and close method would access uninitialized m_reader.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 18, 2015

Member

Hm, you are right. A yield() at the beginning of startReader would fix that, too, with a little less overhead, but ManualEvent (and thus TaskMutex) are supposed to get some single-thread optimizations anyway, so that shouldn't matter. So I'd say your approach is preferable due to the clearer semantics.

Member

s-ludwig commented Dec 18, 2015

Hm, you are right. A yield() at the beginning of startReader would fix that, too, with a little less overhead, but ManualEvent (and thus TaskMutex) are supposed to get some single-thread optimizations anyway, so that shouldn't matter. So I'd say your approach is preferable due to the clearer semantics.

s-ludwig added a commit that referenced this pull request Dec 18, 2015

Merge pull request #1356 from 9il/patch-4
Fix high-level race-conditions in WebSocket's initialization code. Fixes #1354.

@s-ludwig s-ludwig merged commit e82e6d5 into vibe-d:master Dec 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@9il 9il deleted the 9il:patch-4 branch Dec 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment