Closed bnoordhuis closed 10 years ago
Nice cleanup. Thanks. The code LGTM.
Cheers, landed. For posterity, one of the lifecycle bugs that's still at large:
==9002== Invalid write of size 4
==9002== at 0x5A6C050: pthread_mutex_lock (pthread_mutex_lock.c:161)
==9002== by 0xBA1C73A: ssOswPthread_mutex_lock (in /home/bnoordhuis/opt/oci/libclntshcore.so.12.1)
==9002== by 0xBA1C94E: sltsmna (in /home/bnoordhuis/opt/oci/libclntshcore.so.12.1)
==9002== by 0x8D4B02B: kpultfr (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0x8C0B51C: OCILobFreeTemporary (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0xAF259B7: oracle::occi::ConnectionImpl::freeTemporaryLob(oracle::occi::EnvironmentImpl*, OCILobLocator*) const (in /home/bnoordhuis/opt/oci/libocci.so.12.1)
==9002== by 0xAF4EE0F: oracle::occi::Clob::~Clob() (in /home/bnoordhuis/opt/oci/libocci.so.12.1)
==9002== by 0x786E89C: output_t::~output_t() (in /home/bnoordhuis/src/strongloop/strong-oracle/build/Debug/oracle_bindings.node)
==9002== by 0x786DFC9: ExecuteBaton::ResetOutputs() (executeBaton.cpp:182)
==9002== by 0x786D436: ExecuteBaton::~ExecuteBaton() (executeBaton.cpp:32)
==9002== by 0x785F355: Connection::EIO_AfterExecute(uv_work_s*, int) (connection.cpp:887)
==9002== by 0x9589E0: uv__work_done (threadpool.c:220)
==9002== by 0x94ED08: uv__async_event (async.c:80)
==9002== by 0x94EDD2: uv__async_io (async.c:151)
==9002== by 0x95B72B: uv__io_poll (linux-core.c:263)
==9002== by 0x94F66B: uv_run (core.c:317)
==9002== Address 0x69735b8 is 8 bytes inside a block of size 192 free'd
==9002== at 0x4C28577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9002== by 0xBA1C71A: ssMemFree (in /home/bnoordhuis/opt/oci/libclntshcore.so.12.1)
==9002== by 0xB95CF53: sltsmxd (in /home/bnoordhuis/opt/oci/libclntshcore.so.12.1)
==9002== by 0xB996248: ltsmxd (in /home/bnoordhuis/opt/oci/libclntshcore.so.12.1)
==9002== by 0x7FA3073: kpuiDelMutex (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0x7F9F31F: kpufhndl0 (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0x7F9D4CC: kpufhndl (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0x8D9D34E: kpuspsessionrelease (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0x8C0C4C8: OCISessionRelease (in /home/bnoordhuis/opt/oci/libclntsh.so.12.1)
==9002== by 0xAF26B31: oracle::occi::ConnectionImpl::do_destroy(unsigned int, void*, unsigned int) (in /home/bnoordhuis/opt/oci/libocci.so.12.1)
==9002== by 0xAF25155: oracle::occi::ConnectionImpl::destroy(unsigned int, std::string const&) (in /home/bnoordhuis/opt/oci/libocci.so.12.1)
==9002== by 0xAF1FC68: oracle::occi::EnvironmentImpl::terminateConnection(oracle::occi::Connection*) (in /home/bnoordhuis/opt/oci/libocci.so.12.1)
==9002== by 0x785CAD4: Connection::closeConnection() (connection.cpp:444)
==9002== by 0x785E4F1: Connection::EIO_Close(uv_work_s*) (connection.cpp:720)
==9002== by 0x95864C: worker (threadpool.c:74)
==9002== by 0x94E008: uv__thread_start (uv-common.c:322)
Clearly, an execute() and a close() are crossing each other. TBI.
Fixes many memory leaks and use-after-free bugs.
There are still lifeycle bugs left, the test suite hits them fairly reliably. I wonder if it isn't less work to simply rewrite from scratch instead of trying to track down bug after bug...