search

cinemast / libjson-rpc-cpp

C++ framework for json-rpc (json remote procedure call)
MIT License
955 stars 320 forks source link

Add ability to set time in ListenLoop #305

Closed KaustubhShamshery closed 3 years ago

KaustubhShamshery commented 3 years ago

For some purposes the 1 millisecond wait time might be too much, hence, keeping the default value same (1000 microseconds), add a setter which helps us to set wait time accordingly.

laudrup commented 3 years ago

While this might work, I really think this code is wrong. Not the patch, but the code being patched.

Active polling with sleep is really not the right way to wait for events for many reasons, so I think this code should be rewritten.

Then again, if I'm not mistaken using a raw TCP socket for jsonrpc is not really standard (which is mainly what this is used for I think), so I honestly think all of this should be removed.

As you already mentioned @cinemast this code could really use some love :-)

KaustubhShamshery commented 3 years ago

@laudrup I agree, perhaps you could make it a blocking socket there and then we would not need to wait at all. In other words, CheckForConnection could be a blocking call.

laudrup commented 3 years ago

@KaustubhShamshery I really think it should be non-blocking with a single blocking call to something like poll to wait for events and handle them when they actually happen.

Of course, that would mean that it would still be a blocking API, so an ideal design would allow for that while allowing the user to retrieve a descriptor/handle to use for notifications with the rest of the users application.