Skip to content

fix issue 13262 - Cannot send certain shared data to another thread#5694

Merged
wilzbach merged 2 commits intodlang:masterfrom
schveiguy:fix13262
Aug 21, 2017
Merged

fix issue 13262 - Cannot send certain shared data to another thread#5694
wilzbach merged 2 commits intodlang:masterfrom
schveiguy:fix13262

Conversation

@schveiguy
Copy link
Member

There were 2 major issues:

  1. Variant was not properly removing the shared qualifier when copying data from the stack into the payload (this is always safe, as the Variant itself is not shared, so the payload, which is never accessible by reference, is also not actually shared).
  2. std.traits.ImplicitConversionTargets incorrectly returned const(T)[] as a valid conversion target for shared(T)[]. It should have been const(shared(T))[].

All the tests for the bug were ported to verifying Variant could accept the types, and this in turn fixes the send and receive functions (I added one unit test to verify the original test case to std/concurrency.d).

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @schveiguy!

Bugzilla references

Auto-close Bugzilla Description
13262 Cannot send certain shared data to another thread

send/receive. Also now allows all types of shared items to be stored in
a Variant.
@MetaLang
Copy link
Member

Variant was not properly removing the shared qualifier when copying data from the stack into the payload (this is always safe, as the Variant itself is not shared, so the payload, which is never accessible by reference, is also not actually shared).

What if the user creates a shared Variant?

@schveiguy
Copy link
Member Author

What if the user creates a shared Variant?

Then nothing will work I think :)

@schveiguy
Copy link
Member Author

schveiguy commented Aug 16, 2017

In other words, if you create a shared(Variant), you have to cast away shared to do any operations, so you better know what you are doing.

fun!(S3)(v);
fun!(shared(S3))(v);

// ensure structs that are shared, but don't have shared postblits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to ensure that a struct that does have a shared postblit can be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried. It can't, but not because of this code, there are a whole bunch of places where it fails. I didn't want to fix everything 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough :)

@atilaneves
Copy link
Contributor

LGTM

@schveiguy
Copy link
Member Author

I've updated to avoid using typeid's destroy and postblit anywhere, since these do not take into account whether the postblit and destructor are properly attributed. This should eliminate some invalid types from being incorrectly used.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wilzbach wilzbach merged commit e2a16cc into dlang:master Aug 21, 2017
@schveiguy schveiguy deleted the fix13262 branch August 21, 2017 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants