Repository: dotnet/runtime
File: src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs (C#)

Code before:

        public override bool IsInvalid
        {
            get { return handle == IntPtr.Zero; }
        }

        /// <summary>
        /// Create another instance of SafeEvpPKeyHandle which has an independent lifetime
        /// from this instance, but tracks the same resource.
        /// </summary>
        /// <returns>An equivalent SafeEvpPKeyHandle with a different lifetime</returns>
        public SafeEvpPKeyHandle DuplicateHandle()
        {
            if (IsInvalid)
                throw new InvalidOperationException(SR.Cryptography_OpenInvalidHandle);

            SafeEvpPKeyHandle safeHandle = new SafeEvpPKeyHandle();

            // Keep the source handle alive so that a concurrent Dispose on another
            // thread does not zero the handle field between UpRef and the copy below.
            bool addedRef = false;

            try
            {
                DangerousAddRef(ref addedRef);

                IntPtr handleSnapshot = DangerousGetHandle();
                IntPtr extraHandleSnapshot = ExtraHandle;

                int uprefResult = Interop.Crypto.UpRefEvpPkey(this);

                if (uprefResult != 1)
                {
                    Debug.Fail("Called UpRefEvpPkey on a key which was already marked for destruction");
                    throw Interop.Crypto.CreateOpenSslCryptographicException();
                }

                // Use the captured snapshots so that a concurrent Dispose
                // cannot cause us to copy a zeroed-out handle value.
                safeHandle.SetHandle(handleSnapshot);
                // ExtraHandle is upref'd by UpRefEvpPkey
                safeHandle.ExtraHandle = extraHandleSnapshot;
            }
            catch
            {
                safeHandle.Dispose();
                throw;
            }
            finally
            {
                if (addedRef)

Review comment (suggestion):
We need to preserve throwing `CreateOpenSslCryptographicException`. Consider creating nested try/catch/finally blocks so the try is distinct from calling DangerousAddRef and getting the OpenSSL exception.

We should have no `catch` for the DangerousAddRef, only a finally, to undo it. 

Code after:

        public override bool IsInvalid
        {
            get { return handle == IntPtr.Zero; }
        }

        /// <summary>
        /// Create another instance of SafeEvpPKeyHandle which has an independent lifetime
        /// from this instance, but tracks the same resource.
        /// </summary>
        /// <returns>An equivalent SafeEvpPKeyHandle with a different lifetime</returns>
        public SafeEvpPKeyHandle DuplicateHandle()
        {
            if (IsInvalid)
                throw new InvalidOperationException(SR.Cryptography_OpenInvalidHandle);

            // Keep the source handle alive so that a concurrent Dispose on another
            // thread does not zero the handle field between UpRef and the copy below.
            bool addedRef = false;

            try
            {
                DangerousAddRef(ref addedRef);

                // Reliability: Allocate the SafeHandle before calling UpRefEvpPkey so
                // that we don't lose a tracked reference in low-memory situations.
                SafeEvpPKeyHandle safeHandle = new SafeEvpPKeyHandle();

                int success = Interop.Crypto.UpRefEvpPkey(this);

                if (success != 1)
                {
                    Debug.Fail("Called UpRefEvpPkey on a key which was already marked for destruction");
                    Exception e = Interop.Crypto.CreateOpenSslCryptographicException();
                    safeHandle.Dispose();
                    throw e;
                }

                // Since we didn't actually create a new handle, copy the handle
                // to the new SafeHandle. DangerousAddRef prevents ReleaseHandle
                // from being called, so handle and ExtraHandle are stable here.
                safeHandle.SetHandle(handle);
                // ExtraHandle is upref'd by UpRefEvpPkey
                safeHandle.ExtraHandle = ExtraHandle;
                return safeHandle;
            }
            finally
            {
                if (addedRef)
                {
                    DangerousRelease();