Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix #1354 #1356

Merged
merged 1 commit into from
Dec 18, 2015
Merged

fix #1354 #1356

merged 1 commit into from
Dec 18, 2015

Conversation

9il
Copy link
Contributor

@9il 9il commented Dec 17, 2015

No description provided.

@s-ludwig
Copy link
Member

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

@9il
Copy link
Contributor Author

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
Copy link
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.

s-ludwig added a commit that referenced this pull request Dec 18, 2015
@s-ludwig
Merge pull request #1356 from 9il/patch-4
e82e6d5 
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
@9il 9il deleted the patch-4 branch December 18, 2015 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@9il @s-ludwig