Hi,
We have a slapd preoperation + extendedop plugin that largely works well for us. We have recently discovered that it leaks a small amount of memory, which, over time, creates a headache.
I would appreciate advice on the proper way to solve this problem. In short, our code allocates a small bit of state information for each connection. We register a callback with SLAPI_PLUGIN_PRE_UNBIND_FN which cleans up that state information.
However, that function is only invoked for authenticated connections; simple anonymous binds never invoke the unbind callback, and hence the leak. That is, our problem seems to be when the bind is both simple (LDAP_AUTH_NONE) and anonymous (null DN).
If we set bindSubtree to '*' (ALL_DATA), then our unbind function is invoked for the anonymous connections. It is not clear to us what the ramifications are of that setting, and what effect making that change will have on our plugin. Advice would greatly appreciated.
Alternately, inspection of factory.c suggests that slapi_register_object_extension() could be used as a way for creating and destroying the connection specific information we need to track. However, those callbacks appear to be intended only for slapd internal server plugins, not for external plugins. In order to make that work, we would need a way of connecting a Slapi_PBlock to the opaque Connection structure that is passed to the constructor and destructor. I imagine that we could achieve that by modifying the slapi library to expose something like slapi_get_conn_id() which would take the opaque Connection * pointer and return the connection id. My concern is that needing that call is a signal that it's The Wrong Approach (TM).
If it helps, some bits of how we construct our plugin are shared here: https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f
Clue bats welcome; any advice is appreciated.
Cheers,
Jeremy
On 2/1/22 1:37 PM, Jeremy White wrote:
Hi,
We have a slapd preoperation + extendedop plugin that largely works well for us. We have recently discovered that it leaks a small amount of memory, which, over time, creates a headache.
I would appreciate advice on the proper way to solve this problem. In short, our code allocates a small bit of state information for each connection. We register a callback with SLAPI_PLUGIN_PRE_UNBIND_FN which cleans up that state information.
Before looking into rewriting your code with factory extensions, which can be complex, I would try memory leak detection tools like valgrind or ASAN.
To use ASAN you have to build your rpms with the proper flags:
https://www.port389.org/docs/389ds/howto/howto-addresssanitizer.html
Valgrind can be used with an existing rpm, but we don't have any DS specific docs on it. There is a helper script on github you could try:
https://github.com/richm/scripts/blob/master/ns-slapd.sh (requires coping /usr/sbin/ns-slapd to /usr/sbin/ns-slapd.orig). Please look through this scripts and all of its comments to get it working.
https://github.com/richm/scripts/blob/master/valgrind.supp%C2%A0 (to be placed in /tmp)
Now I'm not sure how you deploy 389, but you can build the rpms yourself using rpm.mk from the ds source tree:
/source/389-ds-base/rpm.mk
Edit this file, set ASAN_ON = 1 and BUNDLE_JEMALLOC=0. Then comment the "check" command from the specfile /source/389-ds-base/rpm/389-ds-base.spec.in
Then build the rpms:
SKIP_AUDIT_CI=1 make -f rpm.mk rpms
Or, build it how you always do and add the --enable-asan flag to configure. See the wiki link above.
That all being said we just noticed that not all leaks get caught in ASAN (although ASAN is suppose to be better than Valgrind), so if ASAN doesn't report a leak (whcih we know there is one), then try Valgrind. With both ASAN and Valgrind you must stop the server for the report file to be fully generated.
I would start here and see if you can fix the leak in your existing code before trying to pull in the factory extensions.
HTH,
Mark
However, that function is only invoked for authenticated connections; simple anonymous binds never invoke the unbind callback, and hence the leak. That is, our problem seems to be when the bind is both simple (LDAP_AUTH_NONE) and anonymous (null DN).
If we set bindSubtree to '*' (ALL_DATA), then our unbind function is invoked for the anonymous connections. It is not clear to us what the ramifications are of that setting, and what effect making that change will have on our plugin. Advice would greatly appreciated.
Alternately, inspection of factory.c suggests that slapi_register_object_extension() could be used as a way for creating and destroying the connection specific information we need to track. However, those callbacks appear to be intended only for slapd internal server plugins, not for external plugins. In order to make that work, we would need a way of connecting a Slapi_PBlock to the opaque Connection structure that is passed to the constructor and destructor. I imagine that we could achieve that by modifying the slapi library to expose something like slapi_get_conn_id() which would take the opaque Connection * pointer and return the connection id. My concern is that needing that call is a signal that it's The Wrong Approach (TM).
If it helps, some bits of how we construct our plugin are shared here: https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f
Clue bats welcome; any advice is appreciated.
Cheers,
Jeremy _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
Hi Mark,
Thanks for responding.
On 2/1/22 13:01, Mark Reynolds wrote:
On 2/1/22 1:37 PM, Jeremy White wrote:
Hi,
We have a slapd preoperation + extendedop plugin that largely works well for us. We have recently discovered that it leaks a small amount of memory, which, over time, creates a headache.
I would appreciate advice on the proper way to solve this problem. In short, our code allocates a small bit of state information for each connection. We register a callback with SLAPI_PLUGIN_PRE_UNBIND_FN which cleans up that state information.
Before looking into rewriting your code with factory extensions, which can be complex, I would try memory leak detection tools like valgrind or ASAN.
I'm sorry I didn't make it clear; we understand exactly why the memory is leaking. That is, we were presuming that there would be a call to the PRE_UNBIND_FN in all cases, but that is happening in only some of the cases.
When that function is not called, we fail to release memory, and the leak occurs.
The issue is that in order to solve the problem, I need the slapd server to invoke my code when a connection has ended so that I can clean up. Hence the look at bindSubtree and the factory methods.
Cheers,
Jeremy
To use ASAN you have to build your rpms with the proper flags:
https://www.port389.org/docs/389ds/howto/howto-addresssanitizer.html
Valgrind can be used with an existing rpm, but we don't have any DS specific docs on it. There is a helper script on github you could try:
https://github.com/richm/scripts/blob/master/ns-slapd.sh (requires coping /usr/sbin/ns-slapd to /usr/sbin/ns-slapd.orig). Please look through this scripts and all of its comments to get it working.
https://github.com/richm/scripts/blob/master/valgrind.supp%C2%A0 (to be placed in /tmp)
Now I'm not sure how you deploy 389, but you can build the rpms yourself using rpm.mk from the ds source tree:
/source/389-ds-base/rpm.mk
Edit this file, set ASAN_ON = 1 and BUNDLE_JEMALLOC=0. Then comment the "check" command from the specfile /source/389-ds-base/rpm/389-ds-base.spec.in
Then build the rpms:
SKIP_AUDIT_CI=1 make -f rpm.mk rpms
Or, build it how you always do and add the --enable-asan flag to configure. See the wiki link above.
That all being said we just noticed that not all leaks get caught in ASAN (although ASAN is suppose to be better than Valgrind), so if ASAN doesn't report a leak (whcih we know there is one), then try Valgrind. With both ASAN and Valgrind you must stop the server for the report file to be fully generated.
I would start here and see if you can fix the leak in your existing code before trying to pull in the factory extensions.
HTH,
Mark
However, that function is only invoked for authenticated connections; simple anonymous binds never invoke the unbind callback, and hence the leak. That is, our problem seems to be when the bind is both simple (LDAP_AUTH_NONE) and anonymous (null DN).
If we set bindSubtree to '*' (ALL_DATA), then our unbind function is invoked for the anonymous connections. It is not clear to us what the ramifications are of that setting, and what effect making that change will have on our plugin. Advice would greatly appreciated.
Alternately, inspection of factory.c suggests that slapi_register_object_extension() could be used as a way for creating and destroying the connection specific information we need to track. However, those callbacks appear to be intended only for slapd internal server plugins, not for external plugins. In order to make that work, we would need a way of connecting a Slapi_PBlock to the opaque Connection structure that is passed to the constructor and destructor. I imagine that we could achieve that by modifying the slapi library to expose something like slapi_get_conn_id() which would take the opaque Connection * pointer and return the connection id. My concern is that needing that call is a signal that it's The Wrong Approach (TM).
If it helps, some bits of how we construct our plugin are shared here: https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f
Clue bats welcome; any advice is appreciated.
Cheers,
Jeremy _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject....
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
For my part, I think that you are on the right track and using the factory extension is the right thing to do if you want to be sure not to leak (especially in special error cases) But I fail to understand what your problem is : Everything you need is in slapi-plugin.h. That said, it is poorly documented. here is a simple example about how to use it:
struct myext_data { /* The per connection data that you need to manage */ };
static int myext_objecttype; static int myext_handle;
void *myext_constructor(void *object __attribute__((unused)), void *parent __attribute__((unused))) { struct myext_data *mydata = slapi_ch_calloc(1, sizeof(struct myext_data)); ... return mydata; }
void myext_destructor(void *ext, void *object __attribute__((unused)), void *parent __attribute__((unused))) { struct myext_data *mydata = ext; ... slapi_ch_free((void**)&mydata); }
struct myext_data *myext_getdata(Slapi_PBlock *pb) { void *conn; slapi_pblock_get(pb, SLAPI_CONNECTION, &conn); return slapi_get_object_extension(myext_objecttype, conn, myext_handle); }
while initializing the plugin: slapi_register_object_extension(plugin_name, SLAPI_EXT_CONNECTION, myext_constructor, myext_destructor, &myext_objecttype, &myext_handle);
while removing plugin slapi_unregister_object_extension(plugin_name, SLAPI_EXT_CONNECTION, &myext_objecttype, &myext_handle);
while handling operation use myext_getdata and store the data you need
Regards Pierre
On Tue, Feb 1, 2022 at 8:22 PM Jeremy White jwhite@codeweavers.com wrote:
Hi Mark,
Thanks for responding.
On 2/1/22 13:01, Mark Reynolds wrote:
On 2/1/22 1:37 PM, Jeremy White wrote:
Hi,
We have a slapd preoperation + extendedop plugin that largely works well for us. We have recently discovered that it leaks a small amount of memory, which, over time, creates a headache.
I would appreciate advice on the proper way to solve this problem. In short, our code allocates a small bit of state information for each connection. We register a callback with SLAPI_PLUGIN_PRE_UNBIND_FN which cleans up that state information.
Before looking into rewriting your code with factory extensions, which can be complex, I would try memory leak detection tools like valgrind or ASAN.
I'm sorry I didn't make it clear; we understand exactly why the memory is leaking. That is, we were presuming that there would be a call to the PRE_UNBIND_FN in all cases, but that is happening in only some of the cases.
When that function is not called, we fail to release memory, and the leak occurs.
The issue is that in order to solve the problem, I need the slapd server to invoke my code when a connection has ended so that I can clean up. Hence the look at bindSubtree and the factory methods.
Cheers,
Jeremy
To use ASAN you have to build your rpms with the proper flags:
https://www.port389.org/docs/389ds/howto/howto-addresssanitizer.html
Valgrind can be used with an existing rpm, but we don't have any DS specific docs on it. There is a helper script on github you could try:
https://github.com/richm/scripts/blob/master/ns-slapd.sh (requires coping /usr/sbin/ns-slapd to /usr/sbin/ns-slapd.orig). Please look through this scripts and all of its comments to get it working.
https://github.com/richm/scripts/blob/master/valgrind.supp (to be placed in /tmp)
Now I'm not sure how you deploy 389, but you can build the rpms yourself using rpm.mk from the ds source tree:
/source/389-ds-base/rpm.mk
Edit this file, set ASAN_ON = 1 and BUNDLE_JEMALLOC=0. Then comment the "check" command from the specfile /source/389-ds-base/rpm/389-ds-base.spec.in
Then build the rpms:
SKIP_AUDIT_CI=1 make -f rpm.mk rpmsOr, build it how you always do and add the --enable-asan flag to configure. See the wiki link above.
That all being said we just noticed that not all leaks get caught in ASAN (although ASAN is suppose to be better than Valgrind), so if ASAN doesn't report a leak (whcih we know there is one), then try Valgrind. With both ASAN and Valgrind you must stop the server for the report file to be fully generated.
I would start here and see if you can fix the leak in your existing code before trying to pull in the factory extensions.
HTH,
Mark
However, that function is only invoked for authenticated connections; simple anonymous binds never invoke the unbind callback, and hence the leak. That is, our problem seems to be when the bind is both simple (LDAP_AUTH_NONE) and anonymous (null DN).
If we set bindSubtree to '*' (ALL_DATA), then our unbind function is invoked for the anonymous connections. It is not clear to us what the ramifications are of that setting, and what effect making that change will have on our plugin. Advice would greatly appreciated.
Alternately, inspection of factory.c suggests that slapi_register_object_extension() could be used as a way for creating and destroying the connection specific information we need to track. However, those callbacks appear to be intended only for slapd internal server plugins, not for external plugins. In order to make that work, we would need a way of connecting a Slapi_PBlock to the opaque Connection structure that is passed to the constructor and destructor. I imagine that we could achieve that by modifying the slapi library to expose something like slapi_get_conn_id() which would take the opaque Connection * pointer and return the connection id. My concern is that needing that call is a signal that it's The Wrong Approach (TM).
If it helps, some bits of how we construct our plugin are shared here: https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f
Clue bats welcome; any advice is appreciated.
Cheers,
Jeremy _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives:
https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject....
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
Hi Pierre,
On 2/2/22 11:08, Pierre Rogier wrote:
For my part, I think that you are on the right track and using the factory extension is the right thing to do if you want to be sure not to leak (especially in special error cases) But I fail to understand what your problem is : Everything you need is in slapi-plugin.h. That said, it is poorly documented. here is a simple example about how to use it:
struct myext_data { /* The per connection data that you need to manage */ }; static int myext_objecttype; static int myext_handle; void *myext_constructor(void *object __attribute__((unused)), void *parent __attribute__((unused))) { struct myext_data *mydata = slapi_ch_calloc(1, sizeof(struct myext_data)); ... return mydata; } void myext_destructor(void *ext, void *object __attribute__((unused)), void *parent __attribute__((unused))) { struct myext_data *mydata = ext; ... slapi_ch_free((void**)&mydata); } struct myext_data *myext_getdata(Slapi_PBlock *pb) { void *conn; slapi_pblock_get(pb, SLAPI_CONNECTION, &conn); return slapi_get_object_extension(myext_objecttype, conn, myext_handle);
Aha! Yes, that is a welcome clue bat. I was stuck in my line of thinking and did not reexamine the problem fully. Thanks for the help!
Cheers,
Jeremy
}while initializing the plugin: slapi_register_object_extension(plugin_name, SLAPI_EXT_CONNECTION, myext_constructor, myext_destructor, &myext_objecttype, &myext_handle);
while removing plugin slapi_unregister_object_extension(plugin_name, SLAPI_EXT_CONNECTION, &myext_objecttype, &myext_handle);
while handling operation use myext_getdata and store the data you need
Regards Pierre
On Tue, Feb 1, 2022 at 8:22 PM Jeremy White <jwhite@codeweavers.com mailto:jwhite@codeweavers.com> wrote:
Hi Mark, Thanks for responding. On 2/1/22 13:01, Mark Reynolds wrote: > > On 2/1/22 1:37 PM, Jeremy White wrote: >> Hi, >> >> We have a slapd preoperation + extendedop plugin that largely works >> well for us. We have recently discovered that it leaks a small amount >> of memory, which, over time, creates a headache. >> >> I would appreciate advice on the proper way to solve this problem. In >> short, our code allocates a small bit of state information for each >> connection. We register a callback with SLAPI_PLUGIN_PRE_UNBIND_FN >> which cleans up that state information. > > Before looking into rewriting your code with factory extensions, which > can be complex, I would try memory leak detection tools like valgrind or > ASAN. I'm sorry I didn't make it clear; we understand exactly why the memory is leaking. That is, we were presuming that there would be a call to the PRE_UNBIND_FN in all cases, but that is happening in only some of the cases. When that function is not called, we fail to release memory, and the leak occurs. The issue is that in order to solve the problem, I need the slapd server to invoke my code when a connection has ended so that I can clean up. Hence the look at bindSubtree and the factory methods. Cheers, Jeremy > > To use ASAN you have to build your rpms with the proper flags: > > https://www.port389.org/docs/389ds/howto/howto-addresssanitizer.html <https://www.port389.org/docs/389ds/howto/howto-addresssanitizer.html> > > Valgrind can be used with an existing rpm, but we don't have any DS > specific docs on it. There is a helper script on github you could try: > > https://github.com/richm/scripts/blob/master/ns-slapd.sh <https://github.com/richm/scripts/blob/master/ns-slapd.sh> (requires > coping /usr/sbin/ns-slapd to /usr/sbin/ns-slapd.orig). Please look > through this scripts and all of its comments to get it working. > > https://github.com/richm/scripts/blob/master/valgrind.supp <https://github.com/richm/scripts/blob/master/valgrind.supp> (to be > placed in /tmp) > > > Now I'm not sure how you deploy 389, but you can build the rpms yourself > using rpm.mk <http://rpm.mk> from the ds source tree: > > /source/389-ds-base/rpm.mk <http://rpm.mk> > > Edit this file, set ASAN_ON = 1 and BUNDLE_JEMALLOC=0. Then comment the > "check" command from the specfile > /source/389-ds-base/rpm/389-ds-base.spec.in <http://389-ds-base.spec.in> > > Then build the rpms: > > SKIP_AUDIT_CI=1 make -f rpm.mk <http://rpm.mk> rpms > > Or, build it how you always do and add the --enable-asan flag to > configure. See the wiki link above. > > That all being said we just noticed that not all leaks get caught in > ASAN (although ASAN is suppose to be better than Valgrind), so if ASAN > doesn't report a leak (whcih we know there is one), then try Valgrind. > With both ASAN and Valgrind you must stop the server for the report file > to be fully generated. > > I would start here and see if you can fix the leak in your existing code > before trying to pull in the factory extensions. > > HTH, > > Mark > >> >> However, that function is only invoked for authenticated connections; >> simple anonymous binds never invoke the unbind callback, and hence the >> leak. That is, our problem seems to be when the bind is both simple >> (LDAP_AUTH_NONE) and anonymous (null DN). >> >> If we set bindSubtree to '*' (ALL_DATA), then our unbind function is >> invoked for the anonymous connections. It is not clear to us what the >> ramifications are of that setting, and what effect making that change >> will have on our plugin. Advice would greatly appreciated. >> >> Alternately, inspection of factory.c suggests that >> slapi_register_object_extension() could be used as a way for creating >> and destroying the connection specific information we need to track. >> However, those callbacks appear to be intended only for slapd internal >> server plugins, not for external plugins. In order to make that work, >> we would need a way of connecting a Slapi_PBlock to the opaque >> Connection structure that is passed to the constructor and destructor. >> I imagine that we could achieve that by modifying the slapi library to >> expose something like slapi_get_conn_id() which would take the opaque >> Connection * pointer and return the connection id. My concern is that >> needing that call is a signal that it's The Wrong Approach (TM). >> >> If it helps, some bits of how we construct our plugin are shared here: >> https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f <https://gist.github.com/jwhite66/128aea1b186eaed4c58accfe30590d5f> >> >> Clue bats welcome; any advice is appreciated. >> >> Cheers, >> >> Jeremy >> _______________________________________________ >> 389-devel mailing list -- 389-devel@lists.fedoraproject.org <mailto:389-devel@lists.fedoraproject.org> >> To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org <mailto:389-devel-leave@lists.fedoraproject.org> >> Fedora Code of Conduct: >> https://docs.fedoraproject.org/en-US/project/code-of-conduct/ <https://docs.fedoraproject.org/en-US/project/code-of-conduct/> >> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines <https://fedoraproject.org/wiki/Mailing_list_guidelines> >> List Archives: >> https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org <https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org> >> >> Do not reply to spam on the list, report it: >> https://pagure.io/fedora-infrastructure <https://pagure.io/fedora-infrastructure> > _______________________________________________ 389-devel mailing list -- 389-devel@lists.fedoraproject.org <mailto:389-devel@lists.fedoraproject.org> To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org <mailto:389-devel-leave@lists.fedoraproject.org> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ <https://docs.fedoraproject.org/en-US/project/code-of-conduct/> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines <https://fedoraproject.org/wiki/Mailing_list_guidelines> List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org <https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure <https://pagure.io/fedora-infrastructure>--
389 Directory Server Development Team
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.... Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
389-devel@lists.fedoraproject.org
-
Jeremy White -
Mark Reynolds -
Pierre Rogier