Skip to content

Conversation

@alexdresko
Copy link
Owner

via zimmer62/HSPI@356e0eb

  • Non-blocking connect support
  • Made some protected properties public

Rather than introduce a new method as @zimmer62 did, I modified the existing Connect method to optionally return immediately. By default, the Connect function works as it always has.

zimmer62@356e0eb

* Non-blocking connect support
* Made some protected properties public
@alexdresko alexdresko self-assigned this Sep 3, 2018
@alexdresko alexdresko requested a review from zshall September 3, 2018 22:51
Justification = "I don't know what kinds of exceptions it _could_ throw.")]
public static void Connect<TPlugin>(string[] args) where TPlugin : HspiBase, new()
public static TPlugin Connect<TPlugin>(string[] args, bool pollForShutdown = true) where TPlugin : HspiBase, new()
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if pollForShutdown is true, the app will exit when the plugin shuts down, and this method will block until it does. And if it isn’t true, this method will return a plug-in instance. Do multiple plugins get loaded at a time? If one of them is polling for shutdown and shuts down then the whole app will exit. Is this intended?

Copy link
Owner Author

Choose a reason for hiding this comment

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

HS does support multiple plugin instances, but that's ultimately going to be different code. The current HSPI code doesn't support multiple instances. Still have to figure out how all that works.

try
{
while (true)
while (pollForShutdown)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, based on the behavior you may want to rename it exitOnShutdown and say if (exitOnShutdown) while true {...} }. It’d make it easier to understand.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Meh, I get what you're saying, but I don't know about exitOnShutdown either. I kinda half-ass threw pollForShutdown on the table just to see if it stuck. Apparently, it didn't. I'll think about it some more before merging, as changing the name later would result in a breaking change.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants