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

Initialize variables in shared static this() #1841

Merged
merged 1 commit into from Jul 14, 2017

Conversation

Projects
None yet
3 participants
@wilzbach
Contributor

wilzbach commented Jul 13, 2017

If connectMongo with SASL (e.g. SCRAM) is called in a static shared constructor, it will nicely segfault.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 13, 2017

Member

Should be both, in shared static this for the main thread, and in static this for other threads. Or, and that's probably the better solution, it could just be created lazily. It currently uses a file descriptor per thread and risks a startup failure even for applications that don't use the SASL authentication.

Member

s-ludwig commented Jul 13, 2017

Should be both, in shared static this for the main thread, and in static this for other threads. Or, and that's probably the better solution, it could just be created lazily. It currently uses a file descriptor per thread and risks a startup failure even for applications that don't use the SASL authentication.

@wilzbach

This comment has been minimized.

Show comment
Hide comment
@wilzbach

wilzbach Jul 13, 2017

Contributor

Or, and that's probably the better solution, it could just be created lazily.

Ok. Should I maybe even move this lazy initialization into cryptorand.d as e.g. sha1HashMixer?

Contributor

wilzbach commented Jul 13, 2017

Or, and that's probably the better solution, it could just be created lazily.

Ok. Should I maybe even move this lazy initialization into cryptorand.d as e.g. sha1HashMixer?

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 13, 2017

Member

That would generally make sense, but I'm not yet sure where the crypto package should be heading, so keeping it private in this module is the safer option for now.

Member

s-ludwig commented Jul 13, 2017

That would generally make sense, but I'm not yet sure where the crypto package should be heading, so keeping it private in this module is the safer option for now.

@s-ludwig s-ludwig added the auto-merge label Jul 13, 2017

@dlang-bot dlang-bot merged commit 7d9cd2d into vibe-d:master Jul 14, 2017

2 checks passed

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

@wilzbach wilzbach deleted the wilzbach:fix-sasl branch Jul 15, 2017

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