Skip to content
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

Threadpool API should use handles, not pointers to structs #1404

Closed
kennykerr opened this issue Dec 9, 2022 · 7 comments
Closed

Threadpool API should use handles, not pointers to structs #1404

kennykerr opened this issue Dec 9, 2022 · 7 comments
Assignees
Labels
usability Touch-up to improve the user experience for a language projection

Comments

@kennykerr
Copy link
Contributor

Types like PTP_WORK, PTP_TIMER, PTP_WAIT, PTP_IO are all handle types (opaque pointers) and should be modeled as such. While there are some Threadpool types that are defined as structs, like TP_CALLBACK_ENVIRON_V3, most should be considered opaque handles.

typedef struct _TP_WORK TP_WORK, *PTP_WORK;

So for example, TP_WORK should not be defined at all. Instead, there should be a PTP_WORK struct with the NativeTypeDef attribute and the InvalidHandleValue attribute.

@kennykerr
Copy link
Contributor Author

All of these have an "invalid handle value" of 0.

@mikebattista mikebattista self-assigned this Dec 12, 2022
@mikebattista mikebattista added the usability Touch-up to improve the user experience for a language projection label Dec 12, 2022
@mikebattista
Copy link
Contributor

Does the below look right? PTP_* are then remapped to the equivalent TP_* with a pointer.

  {
    "Namespace": "Windows.Win32.System.Threading",
    "Name": "TP_IO",
    "ValueType": "IntPtr",
    "InvalidHandleValues": [ 0 ]
  },
  {
    "Namespace": "Windows.Win32.System.Threading",
    "Name": "TP_TIMER",
    "ValueType": "IntPtr",
    "InvalidHandleValues": [ 0 ]
  },
  {
    "Namespace": "Windows.Win32.System.Threading",
    "Name": "TP_WAIT",
    "ValueType": "IntPtr",
    "InvalidHandleValues": [ 0 ]
  },
  {
    "Namespace": "Windows.Win32.System.Threading",
    "Name": "TP_WORK",
    "ValueType": "IntPtr",
    "InvalidHandleValues": [ 0 ]
  },

Windows.Win32.System.Threading.TP_IO :  => [InvalidHandleValue(0),NativeTypedef]    
Windows.Win32.System.Threading.TP_IO.Value added
Windows.Win32.System.Threading.TP_TIMER :  => [InvalidHandleValue(0),NativeTypedef] 
Windows.Win32.System.Threading.TP_TIMER.Value added
Windows.Win32.System.Threading.TP_WAIT :  => [InvalidHandleValue(0),NativeTypedef]  
Windows.Win32.System.Threading.TP_WAIT.Value added
Windows.Win32.System.Threading.TP_WORK :  => [InvalidHandleValue(0),NativeTypedef]  
Windows.Win32.System.Threading.TP_WORK.Value added

@riverar
Copy link
Collaborator

riverar commented Jan 23, 2023

I wouldn't edit the name of these types. The P preface should just be considered historical and unreliable (for these particular types). All devs need to know is that these are just badly named handles (integers).

(In other words, undo the P removal and the rest looks good.)

@kennykerr
Copy link
Contributor Author

Right, the docs only mention the PTP_X names - stick with that.

@mikebattista
Copy link
Contributor

Could you take a look at https://github.com/microsoft/win32metadata/tree/mikebattista/threadingptptypes?

It looks to be consistent now with how PTP_POOL was implemented but given the odd definitions and changes to the [In][Out] attributes in the diff, it needs to be tested to make sure the dev experience is what you expect.

@kennykerr
Copy link
Contributor Author

Sorry, I'm not sure how to check this without a winmd.

@mikebattista
Copy link
Contributor

I sent you one offline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Touch-up to improve the user experience for a language projection
Projects
None yet
Development

No branches or pull requests

3 participants