On Tue, Dec 24, 2019 at 07:21:53PM +0530, Ayush Mittal wrote: > [BUG: 93065] libgomp: destructor missing to delete goacc_cleanup_key > libgomp constructor creates goacc_cleanup_key on dlopen but doesn't delete > key on dlclose. > dlopen and dlclose of libgomp.so exhausts pthread keys, which results in > pthread_key_create failure. > > pthread_key_delete needs to be called by libgomp destructor. > > Signed-off-by: Ayush Mittal <ayus...@samsung.com> > Signed-off-by: Vaneet Narang <v.nar...@samsung.com>
Please provide a ChangeLog entry, see https://gcc.gnu.org/codingconventions.html#ChangeLogs for details. > --- a/libgomp/env.c > +++ b/libgomp/env.c > @@ -1303,7 +1303,10 @@ initialize_env (void) > goacc_runtime_initialize (); > } > > - Please don't remove the form feed character if there is one, though looking at the sources I really don't know what you are patching, the patch can't apply to trunk which has something quite different in there. env.c ends in goacc_runtime_initialize (); goacc_profiling_initialize (); } #endif /* LIBGOMP_OFFLOADED_ONLY */ > +static void __attribute__((destructor)) env_destroy(){ > + goacc_runtime_deinitialize(); > +} Wrong formatting. The function name needs to start a new line, space before (, there should be void in between ()s and { should be on yet another line, indentation is by 2 columns rather than 5 and another space before (. That said, I really don't see the point of adding a destructor function in one file that just calls a function in another file. > /* The public OpenMP API routines that access these variables. */ > > void > diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c > index 42d005d..9bd0c1a 100644 > --- a/libgomp/oacc-init.c > +++ b/libgomp/oacc-init.c > @@ -658,6 +658,16 @@ goacc_runtime_initialize (void) > goacc_host_init (); > } > > +attribute_hidden void > +goacc_runtime_deinitialize() { > + > +#if !(defined HAVE_TLS || defined USE_EMUTLS) > + pthread_key_delete (goacc_tls_key); > +#endif > + pthread_key_delete(goacc_cleanup_key); > + > +} As in, why isn't this function simply static void __attribute__((destructor)) goacc_runtime_deinitialize (void) { #if !(defined HAVE_TLS || defined USE_EMUTLS) pthread_key_delete (goacc_tls_key); #endif pthread_key_delete (goacc_cleanup_key); } (note again formatting issues, including a useless line at the end). I think all those attribute_hidden uses in oacc-init.c are useless, because oacc-int.h wraps all the declarations inside of a hidden block and so all those decls have hidden visibility, but that is a preexisting problem. > --- a/libgomp/oacc-int.h > +++ b/libgomp/oacc-int.h > @@ -94,6 +94,7 @@ goacc_thread (void) > void goacc_register (struct gomp_device_descr *) __GOACC_NOTHROW; > void goacc_attach_host_thread_to_device (int); > void goacc_runtime_initialize (void); > +void goacc_runtime_deinitialize (void); > void goacc_save_and_set_bind (acc_device_t); > void goacc_restore_bind (void); > void goacc_lazy_initialize (void); And if goacc_runtime_deinitialize is static, it can't be declared here. Jakub
