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 #1343 - re-enable std.concurrency integration #1345

Merged
merged 6 commits into from Jan 29, 2016

Conversation

Projects
None yet
2 participants
@s-ludwig
Member

s-ludwig commented Dec 2, 2015

No description provided.

@Geod24

This comment has been minimized.

Show comment
Hide comment
@Geod24

Geod24 Dec 2, 2015

Contributor

Moving from d8b51c2

I've reverted the changes and will start a branch. I didn't realize that the dependencies had become so deep in the meantime.

Had the same experience a few weeks ago. Also, one question is, are we absolutely certain that the scheduler will be set first ? What if the user doesn't import any vibe.d module from his main (he could use extern functions, and have the implementation in other modules. Would the shared ctor still run first ?

Contributor

Geod24 commented Dec 2, 2015

Moving from d8b51c2

I've reverted the changes and will start a branch. I didn't realize that the dependencies had become so deep in the meantime.

Had the same experience a few weeks ago. Also, one question is, are we absolutely certain that the scheduler will be set first ? What if the user doesn't import any vibe.d module from his main (he could use extern functions, and have the implementation in other modules. Would the shared ctor still run first ?

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 3, 2015

Member

That should be the case, since the module constructor would still be in the dependency chain. Not sure if dynamic libraries work correctly, but since the initialization happens in vibe.core.core, otherwise this would not only break the std.concurrency integration, but all vibe.d core functionality.

Member

s-ludwig commented Dec 3, 2015

That should be the case, since the module constructor would still be in the dependency chain. Not sure if dynamic libraries work correctly, but since the initialization happens in vibe.core.core, otherwise this would not only break the std.concurrency integration, but all vibe.d core functionality.

s-ludwig added some commits Dec 2, 2015

Keep using vibe.d's std.concurrency implementation internally.
There are some unsupported features in std.concurrency that makes it impossible to use for some current use cases:
- Non-copyable objects cannot be passed to spawned threads
- Tasks started with runTask and runWorkerTask cannot be used as message receivers
Add workaround for libasync test assertion failure.
std.concurrency caused a driver instance to be created in a libasync control thread.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 29, 2016

Member

I just went over the source again and all existing places use vibe.d's own "...Compat" version of the message passing code, so there should be no functional change there. Since the tests are green now, including the message passing tests, I'll merge this now and tag a new pre-release for 0.7.27.

Member

s-ludwig commented Jan 29, 2016

I just went over the source again and all existing places use vibe.d's own "...Compat" version of the message passing code, so there should be no functional change there. Since the tests are green now, including the message passing tests, I'll merge this now and tag a new pre-release for 0.7.27.

s-ludwig added a commit that referenced this pull request Jan 29, 2016

Merge pull request #1345 from rejectedsoftware/fix_1343
Fix #1343 - re-enable std.concurrency integration

@s-ludwig s-ludwig merged commit 8257198 into master Jan 29, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@s-ludwig s-ludwig deleted the fix_1343 branch Jan 29, 2016

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