Skip to content

Conversation

@kinke
Copy link
Contributor

@kinke kinke commented Oct 1, 2017

The Key and {Key,Value}NameSequence objects weren't referenced during and after iteration in ...NameSequence.opApply(), so there was no guarantee the Key object owning the HKEY handle would stay alive and keep the handle open - its destructor closes the handle.

This lead to a failing std.datetime.timezone unittest for optimized LDC builds.

Pinging @klickverbot @ZombineDev @rainers as involved in the discussion starting here.

The `Key` and `{Key,Value}NameSequence` objects weren't referenced during
and after iteration in `...NameSequence.opApply()`, so there was no
guarantee the `Key` object owning the HKEY handle would stay alive and
keep the handle open - its destructor closes the handle.

This lead to a failing std.datetime.timezone unittest for optimized LDC
builds.
@kinke kinke requested a review from CyberShadow as a code owner October 1, 2017 14:20
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Copy link
Member

@CyberShadow CyberShadow left a comment

Choose a reason for hiding this comment

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

Thanks!

}

private void regProcessNthKey(HKEY hkey, scope void delegate(scope LONG delegate(DWORD, out string)) dg)
private void regProcessNthKey(Key key, scope void delegate(scope LONG delegate(DWORD, out string)) dg)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anything blocking? ;) - I'm not sure whether decorating the key param with scope makes a difference.

@dlang-bot dlang-bot merged commit 0bdd723 into dlang:stable Oct 5, 2017
@kinke
Copy link
Contributor Author

kinke commented Oct 5, 2017

Thanks. :)

@kinke kinke deleted the winref branch October 5, 2017 21:45
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.

3 participants