Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Moving SafeDeleteContext and SafeFreeCredentials to common location#8452

Merged
bartonjs merged 1 commit into
dotnet:masterfrom
kapilash:relocate
May 19, 2016
Merged

Moving SafeDeleteContext and SafeFreeCredentials to common location#8452
bartonjs merged 1 commit into
dotnet:masterfrom
kapilash:relocate

Conversation

@kapilash
Copy link
Copy Markdown
Contributor

@kapilash kapilash commented May 11, 2016

SafeFreeCredentials and SafeDeleteContext are used across two native libraries - libssl and System.Net.Security.Native.
Moving these two classes a common location.

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 11, 2016

This is in response to a PR comment received

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 11, 2016

cc: @stephentoub

@stephentoub
Copy link
Copy Markdown
Member

stephentoub commented May 11, 2016

cc: @bartonjs

bartonjs
bartonjs reviewed May 11, 2016
View reviewed changes
Comment thread src/Common/src/Interop/Unix/libssl/SecuritySafeHandles.cs Outdated
}
}

internal sealed class SafeDeleteSslContext : SafeDeleteContext
Copy link
Copy Markdown
Member

@bartonjs bartonjs May 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything left in this file (for example, this class) should be also moved to new files so we can delete the whole libssl directory

Copy link
Copy Markdown
Contributor Author

@kapilash kapilash May 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split the file into different classes. I moved the classes which are under System.Net.Security namespace to $(COMMONPATH)\System\Net\Security\Unix and those in default namespace (Interop.*) to $(COMMONPATH)\Interop\Unix\System.Security.Cryptography.Native
Please let me know if you have any other location in mind

@stephentoub
Copy link
Copy Markdown
Member

stephentoub commented May 13, 2016

Test Innerloop CentOS7.1 Debug Build and Test please

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 13, 2016

@dotnet-bot test outerloop rhel7.2 release

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 14, 2016

Outerloop test failure is due to

    08:38:16 Unhandled Exception: System.InvalidOperationException: This operation is only allowed using a successfully authenticated context.
    08:38:16    at System.Net.Security.SslState.CheckThrow(Boolean authSucessCheck)
    08:38:16    at System.Net.Security.SslState.get_SecureStream()
    08:38:16    at System.Net.Security.SslStream.Read(Byte[] buffer, Int32 offset, Int32 count)
    08:38:16    at System.IO.Stream.<>c.<BeginReadInternal>b__38_0(Object <arg>)
    08:38:16    at System.Threading.Tasks.Task`1.InnerInvoke()
    08:38:16    at System.Threading.Tasks.Task.Execute()
    08:38:16 --- End of stack trace from previous location where exception was thrown ---
    08:38:16    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
    08:38:16    at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
    08:38:16    at System.IO.Stream.EndRead(IAsyncResult asyncResult)
    08:38:16    at System.Net.Security.Tests.DummyTcpServer.OnReceive(IAsyncResult result)
    08:38:16    at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
    08:38:16    at System.IO.Stream.ReadWriteTask.System.Threading.Tasks.ITaskCompletionAction.Invoke(Task completingTask)
    08:38:16    at System.Threading.Tasks.Task.FinishContinuations()
    08:38:16    at System.Threading.Tasks.Task.Finish(Boolean bUserDelegateExecuted)
    08:38:16    at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
    08:38:16    at System.Threading.Tasks.Task.ExecuteEntry(Boolean bPreventDoubleExecution)
    08:38:16    at System.Threading.ThreadPoolWorkQueue.Dispatch()
    08:38:30 ./RunTests.sh: line 198: 33435 Aborted                 (core dumped) ./corerun xunit.console.netcore.exe System.Net.Security.Tests.dll -xml testResults.xml -notrait Benchmark=true -notrait category=IgnoreForCI -notrait category=failing -notrait category=nonlinuxtests

I think this is same as #8472 (which should get fixed with new packages)

bartonjs
bartonjs reviewed May 15, 2016
View reviewed changes
Comment thread src/Common/src/System/Net/Security/Unix/SafeFreeCertContext.cs Outdated
using System.Diagnostics;
using System.Runtime.InteropServices;
//using System.Security.Authentication;
//using System.Security.Authentication.ExtendedProtection;
Copy link
Copy Markdown
Member

@bartonjs bartonjs May 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem like they could be deleted...

Copy link
Copy Markdown
Contributor Author

@kapilash kapilash May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you. I missed these. Removed them now

@kapilash
clean src\Common\src\Interop\Unix\libssl directory
76eb58c 
* Corefx does not pinvoke into libssl directly anymore. This commit moves the Interop files in $(COMMONPATH)\Interop\Unix\libssl to $(COMMONPATH)\Interop\Unix\System.Security.Cryptography.Native directory. Likewise, the SecuritySafeHandles are split across different files and are moved to $(COMMONPATH)\System\Net\Security\Unix.
@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 17, 2016

Both the inner loop failures are in System.Runtime.Loader.Tests. Not yet sure why they fail only on CentOS

                        System.Runtime.Loader.Tests.AssemblyLoadContextTest.LoadFromAssemblyName_ValidTrustedPlatformAssembly [FAIL]
      06:51:24       Assert.Throws() Failure
      06:51:24       Expected: typeof(System.IO.FileLoadException)
      06:51:24       Actual:   (No exception was thrown)
      06:51:24       Stack Trace:
      06:51:24             at  System.Runtime.Loader.Tests.AssemblyLoadContextTest.LoadFromAssemblyName_ValidTrustedPlatformAssembly()

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 17, 2016

@dotnet-bot test outerloop rhel7.2 release

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 18, 2016 •
edited
Loading

@dotnet-bot test Innerloop CentOS7.1 debug

@kapilash
Copy link
Copy Markdown
Contributor Author

kapilash commented May 18, 2016

@dotnet-bot test this please

@bartonjs
Copy link
Copy Markdown
Member

bartonjs commented May 19, 2016

Looks a lot better. Thanks, Hemanth.

@bartonjs bartonjs merged commit b020dfd into dotnet:master May 19, 2016
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
@karelz karelz added os-linux Linux OS (any supported distro) and removed X-Plat labels Mar 8, 2017
@kapilash kapilash mentioned this pull request Jan 31, 2020
Closed
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@bartonjs
Merge pull request dotnet/corefx#8452 from kapilash/relocate
eac5aac 
Moving SafeDeleteContext and SafeFreeCredentials to common location

Commit migrated from dotnet/corefx@b020dfd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

area-System.Net os-linux Linux OS (any supported distro)

Projects

None yet

Milestone

1.0.0-rtm

Development

Successfully merging this pull request may close these issues.

7 participants

@kapilash @stephentoub @bartonjs @karelz @davidsh @dotnet-bot @dnfclas