Onion initialization function

14 views
Skip to first unread message

Zachary Grafton

unread,
Apr 16, 2016, 4:17:56 PM4/16/16
to onion-dev
David,

I was wondering if you would be against adding an onion_init function so that we can initialize some internal stuff there. Currently, I'm running a build with clang's thread sanitizer, and I'm getting a data race issue with the logging not being initialized before messages are logged. I don't think it would be reasonable to add a mutex or a lock of any sorts to the logging as that would definitely slow things down. However, I think it'd be reasonable to expect the user to let us initialize some stuff in an initialization function. We might even be able to use this function later on to let the user specify the number of onion objects that are listening so we can write a proper signal handler as well.

Zack

David Moreno Montero

unread,
Apr 16, 2016, 5:37:27 PM4/16/16
to Zachary Grafton, onion-dev

I think we can assume the first function to be called to be some subset of the config functions, as onion_new, onion_url_new or onion_handler_new, so we can do some onion_init there. It should be idempotent so it can be called as many times as desired.

Anyway I would first really check if there is some other way. I thought pthread_init_once did the trick.

Can you send the log output?

Also, for 1.0 maybe pthreads should be mandatory...

David

--
You received this message because you are subscribed to the Google Groups "onion-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to onion-dev+...@coralbits.com.
To post to this group, send email to onio...@coralbits.com.
Visit this group at https://groups.google.com/a/coralbits.com/group/onion-dev/.
For more options, visit https://groups.google.com/a/coralbits.com/d/optout.

Zachary Grafton

unread,
Apr 17, 2016, 7:36:48 AM4/17/16
to onion-dev, zachary...@gmail.com
David,

The problem seems to stem from the adjusting the output of the logging using the environment variables and the library exposing external state through a global variable. More specifically, in response.c at line 179, we are checking onion_log_flags to see if OF_NOINFO is set before onion_init_logging can be run. It's definitely a bug and could be causing a log message to be lost. That leads me to the following conclusions:
  • We could make sure logging is initialized before the server is started using an onion_init function, this wouldn't really have an performance impacts, but it's a little more overhead for the user
  • We could add getters/setters for the onion_log_flags and hide the implementation. This would work pretty well I think, but the getters and setters would have to use pthread_init_once to call the initialization function, which could add a little runtime overhead
  • We could remove the check on the flags and just call the ONION_INFO macro on every response, but we'd pay the runtime overhead of onion_request_get_client_description everytime.
  • We could use somthing like the gcc __attribute__((constructor)) on onion_init_logging to make sure it is called when the library is loaded, but if the user is linking statically, we'd still need to add a reference to the onion_init_logging to make sure it is called, which is similar in approach to just callling onion_init and we'd have to add some linker flags from what I understand
  • The last option that I could see, is to just let the user choose the logging option at compile time for libonion and removing/adding the appropriate logging calls. This would remove the ability to change the logging level after the library is compiled, but it would add no runtime cost at all.
Feedback would be appreciated.

Thanks,
Zack

Zachary Grafton

unread,
Apr 17, 2016, 7:43:57 AM4/17/16
to onion-dev, zachary...@gmail.com
As for making pthreads mandatory in the future, I think that would be reasonable. Most users would expect to be able to use more than one core for application.


On Saturday, April 16, 2016 at 5:37:27 PM UTC-4, David Moreno Montero wrote:
Reply all
Reply to author
Forward
0 new messages