Skip to content
This repository was archived by the owner on Jul 26, 2023. It is now read-only.

Kernel32 Threading API #463

Merged
merged 12 commits into from
Jul 3, 2020
Merged

Kernel32 Threading API #463

merged 12 commits into from
Jul 3, 2020

Conversation

@vatsan-madhavan
Copy link
Member

@vatsan-madhavan vatsan-madhavan commented Jun 27, 2020 •
edited
Loading

  • CreateThread, CreateRemoteThread, CreateRemoteThreadEx
  • CreateThreadFlags enum
  • Tests
  • FriendlyFlags.NativeInt flag applicable to IntPtr and UIntPtr types that are used as a proxy for Win32 SIZE_T type.
    • A future update will add code-gen for generating appropriate overloads leverage C# 9's nint/nuint.
      • We need an issue for this.
    • We probably also need an issue to inventory methods that have parameters or return-types that logically map to Wn32 SIZE_T and tag them with FriendlyFlag.NativeInt.

Fixes #428

@vatsan-madhavan vatsan-madhavan marked this pull request as draft June 27, 2020 21:00
@vatsan-madhavan vatsan-madhavan mentioned this pull request Jul 1, 2020
Closed
jkotas
jkotas reviewed Jul 1, 2020
View reviewed changes
jkotas
jkotas reviewed Jul 1, 2020
View reviewed changes
jkotas
jkotas reviewed Jul 1, 2020
View reviewed changes
AArnott
AArnott suggested changes Jul 1, 2020
View reviewed changes
Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

@AArnott AArnott mentioned this pull request Jul 1, 2020
Closed
@vatsan-madhavan
- Remove SafeThreadHandle - SafeObjectHandle has the same functionality.
7bdb411 
- Remove SIZE_T - use UIntPtr instead; C# 9 can leverage nuint.
- Address review feedback
  - Replace IntPtr parameters with pointers.
  - Use FriendlyFlags.Optional where appropriate.
- Update tests.
@vatsan-madhavan vatsan-madhavan marked this pull request as ready for review July 2, 2020 03:56
@vatsan-madhavan
Copy link
Member Author

Addressed comments so far. Outstanding at this time:

  • Should we keep using CreateProcessFlags or create a new CreateThreadFlags enum
  • Add FriendlyFlags.NativeInt for future use.
    • Bring back SIZE_T if possible to write one reliably.

@vatsan-madhavan
Copy link
Member Author

vatsan-madhavan commented Jul 2, 2020 •
edited
Loading

I couldn't tag void* parameters with FriendlyFlags.Optional or FriendlyFlags.Bidirectional. I'm assuming that this is normal/expected?

@AArnott
Copy link
Collaborator

AArnott commented Jul 2, 2020

I couldn't tag void* parameters with FriendlyFlags.Optional or FriendlyFlags.Bidirectional. I'm assuming that this is normal/expected?

True, void* has no non-pointer alternative, so friendly attributes don't work on them.

@AArnott
Copy link
Collaborator

AArnott commented Jul 2, 2020

Should we keep using CreateProcessFlags or create a new CreateThreadFlags enum

Create a new one.

Add FriendlyFlags.NativeInt for future use. Bring back SIZE_T if possible to write one reliably.

Future use option sounds good. The SIZE_T struct doesn't sound enticing given @jkotas' warning.

AArnott
AArnott approved these changes Jul 2, 2020
View reviewed changes
Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Just one question about that test delegate

@AArnott AArnott merged commit 1be11c2 into dotnet:master Jul 3, 2020
@vatsan-madhavan
Copy link
Member Author

Thanks for fixing up the branch and appreciate the feedback + help 👍

@vatsan-madhavan vatsan-madhavan deleted the threadingapi branch July 3, 2020 06:14
@AArnott
Copy link
Collaborator

AArnott commented Jul 3, 2020

Thank you for the submission!

@vatsan-madhavan vatsan-madhavan mentioned this pull request Jul 3, 2020
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@jkotas jkotas jkotas left review comments

@AArnott AArnott AArnott approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing kernel32 threading API

3 participants

@vatsan-madhavan @AArnott @jkotas