New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACE_Singleton has a pointer to a lock which can be used after ACE::fini destroys that lock #554

Closed
mitza-oci opened this Issue Dec 20, 2017 · 4 comments

Comments

2 participants
@mitza-oci
Member

mitza-oci commented Dec 20, 2017

See PR #553 for the test case.

@mitza-oci mitza-oci added the bug label Dec 20, 2017

@mitza-oci mitza-oci self-assigned this Dec 20, 2017

@mitza-oci

This comment has been minimized.

Member

mitza-oci commented Dec 20, 2017

ACE_Singleton<TYPE, ACE_LOCK>::instance () has a local static pointer lock which, after first use, points to an object controlled by ACE_Object_Manager. When ACE_Object_Manager shuts down (ACE::fini), this lock is destroyed. When singleton is used after the next ACE::init, the pointer refers to an invalid object and depending on the lock type is not re-initialized.

@jwillemsen jwillemsen added this to the 6.4.7/2.4.7 milestone Dec 20, 2017

@mitza-oci

This comment has been minimized.

Member

mitza-oci commented Dec 20, 2017

Potential fixes:

  1. in instance(), when registering for cleanup with object manager, pass &lock as the extra parameter to at_exit. Then cleanup(void*) gets this value as the parameter and can zero it out.
  2. ACE_Object_Manager::get_singleton_lock overloads for ACE_Thread_Mutex, ACE_Mutex, ACE_RW_Thread_Mutex should not check for null first (make those behave like ACE_Null_Mutex and ACE_Recursive_Thread_Mutex
@mitza-oci

This comment has been minimized.

Member

mitza-oci commented Dec 20, 2017

bug fix option 1

@@ -103,7 +103,7 @@
               ACE_NEW_RETURN (singleton, (ACE_Singleton<TYPE, ACE_LOCK>), 0);

               // Register for destruction with ACE_Object_Manager.
-              ACE_Object_Manager::at_exit (singleton, 0, typeid (TYPE).name ());
+              ACE_Object_Manager::at_exit (singleton, &lock, typeid (TYPE).name ());
 #if defined (ACE_MT_SAFE) && (ACE_MT_SAFE != 0)
             }
 #endif /* ACE_MT_SAFE */
@@ -114,11 +114,18 @@
 }

 template <class TYPE, class ACE_LOCK> void
-ACE_Singleton<TYPE, ACE_LOCK>::cleanup (void *)
+ACE_Singleton<TYPE, ACE_LOCK>::cleanup (void *param)
 {
   ACE_Object_Manager::remove_at_exit (this);
   delete this;
   ACE_Singleton<TYPE, ACE_LOCK>::instance_i () = 0;
+
+#ifdef ACE_FACE_SAFETY_BASE
+  ACE_UNUSED_ARG (param);
+#else
+  ACE_LOCK **lock = static_cast<ACE_LOCK **> (param);
+  *lock = 0;
+#endif
 }

 template <class TYPE, class ACE_LOCK> void

mitza-oci added a commit to mitza-oci/ACE_TAO that referenced this issue Dec 21, 2017

mitza-oci added a commit that referenced this issue Jan 2, 2018

Merge pull request #555 from mitza-oci/singleton-restart
ACE_Singleton should work after ACE::fini, ACE::init (fixes issue #554)
@jwillemsen

This comment has been minimized.

Member

jwillemsen commented Jan 15, 2018

Can we close this issue?

@mitza-oci mitza-oci closed this Jan 15, 2018

milan-mpathix added a commit to milan-mpathix/ATCD that referenced this issue Dec 5, 2018

Merge tag 'ACE+TAO-6_5_3' into bug_4207
ACE+TAO-6_5_3

* tag 'ACE+TAO-6_5_3':
  ACE+TAO-6_5_3
  List more changes
  Fixed typo in comment
  List new changes
  Remove trailing whitespace per fuzz
  Use correct ACE_Allocator for user-defined data block per API requirements
  Add user-defined ACE_Data_Block test case
  semun is not available on Android with NDK less 12.1b or API level 21. With NDK >= 12.1 or API level >= 21 we include sys/sem.h which defines semun
  Current versions of clang claim to have this fixed, its bugzilla bug is closed
  Removed experiment for using linux/sem.h
  Android API or NDK>=15 do have a senum so don't let ACE define its version
  Revert "Android NDK13 does ship a senum type, isn't usable, but ACE shouldn't…"
  Removed define of ACE_HAS_SEMUN, when not set ACE will define a senum type, it shouldn't do that on Android
  Android NDK13 does ship a senum type, isn't usable, but ACE shouldn't define its own version
  Only define ACE_HAS_SEMUN when we have API >= 12 and at least NDK 15, if not, we just not define it
  Fixed typo
  Don't use sudo anymore as described in https://blog.travis-ci.com/2018-10-04-combining-linux-infrastructures
  Doxygen changes and use const int for the statics to resolve the issues with Embarcadero C++ Builder pre compiled header support
  Doxygen changes
  Add vcpkg with xercesc and openssl to Visual Studio 2017 jobs on Azure Devops
  Use SSL_INCDIR and SSL_LIBDIR as they are now generated by MPC
  Don't compile the ACE tests with clang7 on travisci, it is taking to long
  Revert changes, travis sets CXX before the job starts overruling our settings
  No clang compilation for the moment on azure, not working yet
  Try without clang repo
  Changed optional repo usage
  Fixed indent
  Add optional clang repositories
  Added clang 5/6/7 to azure devops testing
  No need to specify a maxparallel, if not specified it is unlimited
  Add gcc6/7/8 on Linux
  Use gcc8 as default on Linux
  Use CC and CXX instead of COMPILER which only set CXX
  Only set CXX and CC when they are not set by the user
  platform_android.GNU: Fix android_neon
  TAO-INSTALL.html: Formerly not Formally
  ACE-INSTALL.html: Update Android Section
  config-android.h: Adjust a NDK version macro
  TAO-INSTALL.html: Rephrasing
  platform_android.GNU: Combine arm7 marcos
  Add TIE test, bugzilla 4182
  Fixed IOR file usage in test script
  Removed obsolete files
  Use different certificates
  Enable ssliop collocated invocation support
  Added security libs to linker and use certs from another test which do work
  Layout changes
  Layout changes
  Layout changes
  Layout change
  Only list the invocation which failed without the necesary fix
  Add new bug 4213 unit test
  Fixed typos
  New unit test for bugzilla 4213
  Fixed link to OMG ORT spec
  Fixed incorrect classname in logging statement
  Use bool instead of int
  Fixed typos
  Removed commented out operation
  Make formatting of debug message consistent with others in the TAO core
  Removed empty lines
  config-android.h: Fix error message
  platform_android.GNU: Force clang by default
  Error if Android NDK < r15 and Android API >= 21
  Force Non-Versioned SOs on Android
  Remove quotes around compiler env variable
  Use clang7 as compiler
  No .0 minor version for clang7
  Added clang7
  Layout changes
  Add xerces3 and ssl to azure linux job
  Try to fix semaphores in older Android NDKs
  config-android.h: Update for Newer NDK
  Update platform make files to support android clang
  Add ACE/build and TAO/build to gitignore
  Const enhancements
  Add missing configurations to core compilation
  Fixed error
  Compile more msvc2017 configurations and simplified quoting
  Corrected config file
  Added MacOSX job
  clang 7
  90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts 90 min job timeouts
  Add missing -C for gnumake
  Start with perl installation
  Fixed path
  Make more use of environment variables
  Use environment variables for invoking mpc
  Corrected paths
  Fix jobs error
  Make use of vs2017/vs2015/linux on azure
  When we have an error parsing our GIOP header we should log a message using an error, not debug
  Fixed typo
  Removed broken links
  Removed broken link
  Upgraded to latest doxygen configuration file version
  Specify timeout wiht queue and not pool
  Set maximum job timeout for 90 minutes
  Also compile all ACE tests on azure
  Use https links to OBS and remove link to Ken Sedgwick's RPMs because these are not updated anymore
  Make x.5.2 public as new release
  ACE+TAO-6_5_2
  Use const exception data pointer when using invoke, not changed by the implementation, only used
  Updated SSLECName option (see PR DOCGroup#683) for compatibility with OpenSSL libraries built without EC support; applied style guidelines
  Single line clone
  Use depth 1 for MPC
  Use powershell to invoke git clone
  Add steps back as keyword
  Use powershell to create the config.h file
  Fix AIX and Solaris linking rpath errors.
  ACE_QtReactor: work-around a Qt5 bug
  Removed qt4=1 from gnuace.features.
  Added Azure DevOps badge
  Starter azure pipeline definition
  Allow Qt custom file types even when neither supported Qt version is enabled
  Removed comment at top of mpc file
  Layou change
  Small changes to the doc files we package where
  Layout changes
  Use qt5 mpc feature
  Projects using qtreactor need to be skipped when there's no qt
  Removed qt settings to 0, these are disabled by default
  Qt5 support
  Fixed warning from clang
  Fixing warnings from clang by using up-to-date lex and yacc
  Layout changes
  List change
  Layout changes
  Layout changes
  Only log errors when we have a debugging level set
  Layout change
  Fixed typo in comment
  Layout change
  Only define BIO_set_flags when it has not been defined yet
  Layout changes
  Make TAO_Requires_POA_Initializer a const int as attempt to let C++Builder be used with precompiled headers
  Layout changes
  Layout changes
  Some more unicode related changes
  Unicode and layout fixes to the IFR
  Fixed typo in comment
  Corrected version check to also include C++BuilderXE2
  SSLIOP ECDH: Updated NEWS file to include modifications for pull request DOCGroup#683.
  Fixed typo
  Disable Qt5 MPC Feature by Default
  SSLIOP ECDH: Reduced the scope of the variable ec_nid.
  SSLIOP ECDH: Removed id line at top of file.
  SSLIOP ECDH: Updated to execute test using ECDH.
  Added Eliptic Curve support to SSLIOP.
  Use RAND_poll instead of RAND_screen that is deprecated in OpenSSL 1.1
  gnuace: postbuild step shouldn't run until after exeout/dllout/libout link/copy
  Check observer pointer before using it
  Updated msvc check, works now with Visual Studio 2017
  Removed empty lines
  Removed empty line
  Remove old security files
  Layout changes
  Destroy the ORB at the end of main
  List ImR changes
  Const fix
  Active_Pid_Setter is only required on Windows
  Shutdown the ORB at the end of the test
  Make use of std::unique_ptr when C++11 or newer is enabled
  Catch exceptions by reference
  In a per client mode each server only gets one request and it is the responsibility of the client to shutdown the server at that moment
  Fixed incorrect order of arguments to logging macro
  Fixed windows runtime issue
  Store the server status for per client activation within the AAM
  Pass -ORBDebugLevel to the imr with the user specified level (-debug)
  Fixed compile error
  Revert some test changes, the AAM for multiple parallel spawned servers for per client mode will share the same Server_Info struct, aligned logging of the AAM
  Check for self assignment in assignment operator
  Docu change
  Removed get_info method, not used, const changes
  Initialise pointer to zero
  Initialise pointer with zero
  Make use of std::unique_ptr when we have C++11 or newer
  Enhanced logging and use more const
  Fixed typos
  Layout changes
  Pass the server id to the LifeEntry so that we have it in our log
  Logging enhancements
  Logging enhancements
  Fixed typo
  Logging and const changes
  Logging enhancements
  Logging enhancements
  Added ability to control the server activation mode
  Logging enhancements
  Tab changes and add a -imrdebug commandline argument to set the ImR Locator/Activator log level
  Layout changes
  Logging enhancements
  Const and unicode fixes
  Unicode logging fixes
  Logging enhancements
  Indent changes
  Fixed unicode logging error
  Log the POA name when we log that we inform the ImR
  Only log when debug is set and simplify logging of commandline
  Use const for a variable we don't change
  Fixed typo in comment
  Simplified logging
  Updated list of Visual Studio versions we pregenerate as part of the release
  Fix doxygen warnings
  Updated copy/past text for doxygen gen
  Don't ship vc12 generated solutions, Visual Studio 2015 and 2017 are the ones most used
  Make x.5.1 public and prepare for the next release
  ACE+TAO-6_5_1
  Fixed some more links
  Fixed links to the OMG website
  Changed deprecated_declarations to no_deprecated so that we have the same setting with clang and gcc
  Updated changes
  List important changes for the upcoming release
  added explicit include of <string> to fix older MSVC compile errors
  Unicode logging fixes
  When logging that we are a new ORB add the TAO version number so that we can easily determine what TAO version is being used
  add full support for streaming std::(w)string
  Removed references to CIDLC
  Document -GX flag
  Removed duplicated project files (MPC issue 50)
  Removed not used member
  Catch exceptions by reference, not by value
  Moved location of new define
  Set a new define when we use atomic refcount so that we can use that in OpenDDS
  Fix memory leak in schedule; replace broken next-count logic in generate_timer_id
  Remove unused member variable max_per_spoke_
  Revert rename of ref_count_ to refcount_
  Layout change
  Fixed incorrect logging of ascii strings in portableserver
  Fixed another refcount issue where the ec event count was intended
  Fixed compile errors due to rename in the base, make clear that ec has its own refcount
  Make use of std::atomic when C++11 has been enabled
  MacOSX doesn't have a std::atomic_uint32_t
  Make use of std::atomic for the refcount when we have C++11 enabled
  When we have C++11 we are using std::unique_ptr instead of std::auto_ptr
  When using Visual Studio 2017 we can enable C++14 and C++17 compliance, with C++17 we don't have auto_ptr so shouldn't do a using
  List empty PIDL_Files section to not generate DynamicANy.pidl twice
  Document API change
  Add missing include of ace/Auto_Ptr.h
  When we have C++11 ACE_Strong_Bound_Ptr doesn't provide std::auto_ptr support, that is deprecated
  When making a new minor also update micro/beta tags
  Make x.5.0 public
  ACE+TAO-6_5_0
  Removed empty lines
  Fixed typo in comment
  Added starter files for MacOSX Mojave
  Fixed compile errors
  Add unit test for issue DOCGroup#570
  Fix issue DOCGroup#570, crash of TAO_IDL on invalid IDL
  Fixed CodeFactor issues
  Layout changes
  Layout change
  Removed last TAO env macros and some layout changes
  Removed invalid supp
  Next release will be minor
  Updated suppresion     * ACE/bin/valgrind.supp:
  Add valgrind suppression
  List std::unique_ptr usage
  Make use of std::unique_ptr when C++11 is used
  Add clang5 build
  Add g++-6 build
  Also add a gcc7 build
  Change for clang6
  clang6 and gcc8 on travis-ci
  Make use of std::unique_ptr
  Layout changes
  Use std::unique_ptr when we have C++11
  Use std::unique_ptr when we have C++11
  Make use of std::unique_ptr when we have C++11
  Make use of std::unique_ptr when we have C++11
  Use std::unique_ptr when usign C++11
  Make use of std::unique_ptr with C++11
  Use std::unique_ptr with C++11
  Fixed typo in error message
  Further changes for using std::unique_ptr
  No need to initialise auto_ptr/unique_ptr to 0 in the constructor
  Make use of std::unique_ptr when we have C++11
  Make use of std::unique_ptr when we have C++11
  Use std::unique_ptr when using C++11
  Make use of std::unique_ptr when we have C++11
  Make use of std::unique_ptr when we have C++11
  Make use of std::unique_ptr when we have C++11
  When we have C++11 make use of std::unique_ptr
  ACE+TAO x.4.8 release
  ACE+TAO-6_4_8
  List some more changes to ACE
  Document changes
  List changes
  Catch exception by reference
  Get rid of ACE_HAS_NONCONST_FD_ISSET, not used anymore, related to issue DOCGroup#609
  A few more MPC "duplicated files" issues
  Invocation_Retry_Params.h: add required #include
  gnuace MPC type: Environment variables must be "exported" to impact child processes
  GNUAutobuild MPC type:
  Always const_cast for FD_ISSET. Remove intra-statement #if
  Documentation and whitespace changes
  Whitespace changes
  Doxygen fix
  Use true/false instead of 1/0 for connected checks
  Fixed links
  Removed trailing whitespaces and corrected links to bugzilla
  Fixed typo
  Remove obsolete options
  Fixed typos
  Documentation changes, fixed some typos
  Only list VERSION once
  Don't use auto_ptr for the fragmentation strategy, that was the only one that used this in the ORB interface. With C++17 auto_ptr has been removed completely
  Pull changes from master; remove addr_any restriction from ipv6_only UDP
  Combine two logging lines into one
  Don't use inline assembly with clang compilers
  Enable intel assembly when using the Embarcadero clang compilers
  * Fix Handle_Set.inl for Visual Studio
  Add some more error logic for the cases the create_profile fails
  Removed accidental change
  Test double/float as last, they are more complex so could easier fail
  Make use of builtin_bswap(16|32|64)
  * Fix ACE_Handle_Set::is_set to support platforms with both ACE_HAS_BIG_FD_SET and ACE_HAS_NONCONST_FD_ISSET. * Add ACE_HAS_NONCONST_FD_ISSET to compile for recent android NDKs.
  No need to list ace_wchar.inl twice in the MPC file.
  Only set no_deprecated to 1 when it not has been set
  Minor documentation changes and remove old cvs date tags
  Process_Unix.pm: optional stack traces and core file generation for stuck processes
  Compiler_Features_32_Test for clang 6
  Add missing $ prefix to ACE_ROOT
  Fix 64bit issue on load registry data
  Update my e-mail address
  Update instructions for adding a release to openSUSE Build Service
  Fix handling of Debian packaging files in ACE/bin/make_release.py
  Exclude TAO in ACE/debian/ACE-DPKG.mwc
  Include appropriate file from ChangeLogs directory in Debian packages
  Synchronize ACE/debian with contents of Debian source repository
  Remove unused file ACE/rpmbuild/ace-tao-macros.patch
  Improve test for RHEL or CentOS in RPM packaging
  Explicitly require Data::Dumper Perl module to build RPM packages
  Remove workarounds for vc6 and old gcc versions
  Minor improvements to the XML utils classes
  Documentation improvements
  Add get_idref that works wit a string&
  Remove empty ipp files
  Instead of using [0] a [0L] has to be used, that is the correct fix
  ACE/TAO x.4.7 release
  ACE+TAO-6_4_7
  Fix IPv4 case
  TAO test list: mark tests that won't run with CORBA/e compact
  Sun Studio 5.13 and older can't handle this
  TAO NEWS for shmiop change
  Remove preproc checks - if ACE_HAS_IPV6 is set, the IPV^ defs should be there too.
  Check if __GNUC__ is defined and only log that C++ support is ok when the test code has been compiled
  Move issue template to .github
  Remove old left over version commit message
  Minor fixes
  Bug 1220 fix applied to SHMIOP (fixed for IIOP long ago).
  Fixed typo
  Convert to MD format
  Starter issue template
  Any bcc32 version doesn't support inlined assembly
  Bugfix: ACE abort when use %l and the msg len exceed ACE_MAXLOGMSGLEN
  Remove $Id$ to resolve fuzz error
  Add UDP changes and test program
  Added description of changes
  NEWS for upcoming release
  Doxygen fixes and fixed some typos
  List major changes
  Fixed mingw error
  Fixed error
  Fixed errors with versioned namespace builds
  TAO/tests/Bug_3940_Regression/test.idl - Address issue DOCGroup#571 comment by jwillemsen: Disable content of IDL file   for Borland C++ Builder. The cpp32 preprocessor apparently strips out   all @ characters which inhibits processing of annotations.
  Address DOCGroup#571 : Shift handling of annotation applications from lexer to parser.
  msvc7.1 and gcc 4.1.1 can't handle this test so disable this test for those compilers
  Initialise member to solve Codacy warning
  Add constructor to make codacy happy
  Add new compiler test for a C++ feature TAO is using
  Fixed typo in comment
  Convert to doxygen style comments
  Fixed memory leak when we have a wstrval
  Followup to commit 3f751cb addresses DOCGroup#567 (comment) ,
  Fix typos, compile errors
  Fixed compile error
  Pass strings as const&
  Check trailing whitespaces also for .py files
  Mark constructor explicit
  Pull in latest version from XSC which adds a new operation and provides some more information when an error occurs
  Followup to PR DOCGroup#565: Address new conversion warning at   http://buildlogs.remedy.nl/win_msvc15_opendds_debug/index.html , > fe\idl.yy.cpp(1383): warning C4267: 'argument': conversion from > 'size_t' to 'int'
  Add Howard's changes to allow ipv6-only on SOCK_Acceptor
  Follow suggestion by mitza-oci at             DOCGroup#565 : > Since it's a compile-only test, it shouldn't need a run_test.pl or a > test.cpp (build a library instead of an executable).
  Removed old left over cvs id tag
  MPC global.features wireshark2 renamed to wireshark_cmake
  * TAO/tests/Bug_3940_Regression :   New test verifies that tao_idl tolerates IDL4 annotations.
  http://bugzilla.dre.vanderbilt.edu/show_bug.cgi?id=3940#c4 Attachment 1524: Patch to make tao_idl ignore IDL4 annotations
  Removed reference to CIAO, moved to a separate repo, added msvc 14.1 (Visual Studio 2017)
  Compiler Features 32 Test: need to check Apple clang version numbers which use a different scheme than real clang version numbers
  Compiler Features 24 Test: skip older clang versions
  - In section "Building and Installing TAO from git", mention that the   ACE_ROOT and TAO_ROOT env vars are set different from ACE_wrappers. - Fix a few typos. - Add a few missing </LI> tags.
  check clang version number for compiler test workaround
  clang workarounds for Compiler_Features_32_Test
  clang: set ACE_HAS_CPP11 based on predefined macros
  Match MPC for Wireshark 2 Support
  Updated copyright to 2018
  updated test list now that test should pass
  fixed issue DOCGroup#554 ACE_Singleton use after ACE::fini, ACE::init
  Updated MPC project for new ACE test
  New ACE test: check if ACE_Singleton works after ACE::fini, ACE::init
  Added support for FreeBSD 11
  Added support for cross compiling using MinGW on a Linux host.
  Added usage of travis_latest
  Make ACE 6.4.6 and TAO 2.4.6 public
  Fixes corbaloc:uiop:/randesvous|KEY to properly parse KEY instead of |KEY
  Wait for orb thread to terminate
  Fail ssliop_corbaloc test for multi-profile corbaloc
  Fix timeout errors in orbsvcs/tests/Security/ssliop_corbaloc, enable the test
  Fixed ssliop corbaloc parser, when there is a comma delimited list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment