Skip to content
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

[Mono.Debugger.Soft] Connections are not properly closed #7377

Closed
jbevain opened this issue Mar 2, 2018 · 13 comments · Fixed by #7390 or #14773
Assignees
Labels

Comments

@jbevain
Copy link
Member

@jbevain jbevain commented Mar 2, 2018

For a Mono process with the soft debugger on, running on port 4567:

static void Main(string[] args)
{
	var vm = VirtualMachineManager.Connect(new IPEndPoint(IPAddress.Loopback, 4567));
	vm.Detach();

	vm = VirtualMachineManager.Connect(new IPEndPoint(IPAddress.Loopback, 4567));
	vm.Detach();

	Console.ReadLine();
}

We're seeing two threads that are still running and waiting:

threads

Both threads are waiting on Receive with this stacktrace:

Not Flagged		21928	6	Worker Thread	SDB Receiver

System.dll!System.Net.Sockets.Socket.Receive
[Managed to Native Transition]	 
System.dll!System.Net.Sockets.Socket.Receive(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.TcpConnection.TransportReceive(byte[] buf, int buf_offset, int len)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.Receive(byte[] buf, int buf_offset, int len)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.ReadPacket()	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.ReceivePacket()	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.receiver_thread_main()	 
mscorlib.dll!System.Threading.ThreadHelper.ThreadStart_Context(object state)	 
mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	 
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	 
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)	 
mscorlib.dll!System.Threading.ThreadHelper.ThreadStart()	 

The way I look at it in VirtualMachine.Detach

public void Detach () {
	conn.VM_Dispose ();
	conn.Close ();
	notify_vm_event (EventType.VMDisconnect, SuspendPolicy.None, 0, 0, null, 0);
}

VM_Dispose tells the debuggee that we're disposing, and then Connection.Close sets the closed flag to true. Nothing interrupts the receiver_thread_main loop which is waiting.

We've fixed it by switching the calls:

conn.Close ()
conn.VM_Dispose()

This way we're setting the closed flag to true, sending and reading the reply to VM_Dispose, and exiting the receiver_thread_main.

@vargaz, what do you think?

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 2, 2018

Looks good, please send a PR.

@akoeplinger

This comment has been minimized.

Copy link
Member

@akoeplinger akoeplinger commented Mar 22, 2018

Reopening since the PR was reverted.

@akoeplinger akoeplinger reopened this Mar 22, 2018
@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 22, 2018

I think we need another approach to break the read loop in the connection thread.

@sailro

This comment has been minimized.

Copy link
Contributor

@sailro sailro commented Mar 23, 2018

@vargaz do you have more information to share about those random crashes allegedly caused by the previous update?

We have this 'fix' in our own Mono.Debugger.Soft.dll that we ship along with VSTU (for VS on Windows only) from the very beginning (<2014), and I'm not sure that Unity detected crashes about this. cc @joncham.

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 23, 2018

It is definitely caused by this patch. It can be reproduced by running make check in mcs/class/Mono.Debugger.Soft on osx. The tests will not fail, because the crash happens during shutdown, but you can see the native stack traces etc.
If I have to guess, sending the VM_Disconnect message after closing the connection somehow confuses the debugger agent code.

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 23, 2018

Its possible that there were recent changes in mono which caused this to surface, so older runtimes would work fine.

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Mar 25, 2018

It looks like an interaction with 540ef38. The connection is broken, we hit this code:

if (!attach_failed && command_set == CMD_SET_VM && command == CMD_VM_DISPOSE && !(vm_death_event_sent || mono_runtime_is_shutting_down ())) {
	DEBUG_PRINTF (2, "[dbg] Detached - restarting clean debugger thread.\n");
	start_debugger_thread ();
}

so we recursively create new debugger threads which race with the runtime shutdown, leading to all kinds of assertions/crashes.

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Apr 18, 2018

@vargaz what's the resolution on this issue?

@vargaz

This comment has been minimized.

Copy link
Member

@vargaz vargaz commented Apr 18, 2018

Its still open since the proposed fix had to be reverted.

@marek-safar marek-safar removed this from the 2018-02 (5.12.xx) milestone Apr 18, 2018
thaystg added a commit to thaystg/mono that referenced this issue Jun 3, 2019
Couldn't reproduce the same side effect as that caused before.

This PR fixes mono#7377
thaystg added a commit that referenced this issue Jun 6, 2019
This PR fixes #7377
monojenkins added a commit to monojenkins/mono that referenced this issue Jun 10, 2019
thaystg added a commit that referenced this issue Jun 12, 2019
…that caused before. (#14969)

This PR fixes #7377
@marek-safar marek-safar added this to the 2019-06 (6.4.xx) milestone Jun 24, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 6, 2019
Changes: http://github.com/mono/mono/compare/2c3aeaf3780de7392a0d3cbe4dcf86846eb4dffa...29b1ac19c961b959a09097dbc0fe4cd567cc5298

	$ git diff --shortstat 2c3aeaf3..29b1ac19
	 1528 files changed, 45421 insertions(+), 21967 deletions(-)

Changes: mono/api-doc-tools@d03e819...5da8127

	$ git diff --shortstat d03e8198..5da8127a
	 1001 files changed, 86168 insertions(+), 11863 deletions(-)

Changes: mono/api-snapshot@e09042d...1ca8e82

	$ git diff --shortstat e09042da..1ca8e82f
        28 files changed, 612 insertions(+), 217 deletions(-)

Changes: mono/cecil@a6c8f5e...cb6c1ca

	$ git diff --shortstat a6c8f5e1..cb6c1ca9
	 13 files changed, 233 insertions(+), 88 deletions(-)

Changes: mono/corefx@4806207...470e0e1

	$ git diff --shortstat 4806207f...470e0e10
	 4 files changed, 31 insertions(+), 12 deletions(-)

Changes: mono/linker@ebe2a1f...1f87de3

	$ git diff --shortstat ebe2a1f4...1f87de35
        90 files changed, 3219 insertions(+), 1224 deletions(-)

Changes: xamarin/java.interop@befdbc0...be6048e

	$ git diff --shortstat befdbc03...be6048ed
        3 files changed, 3 insertions(+), 3 deletions(-)

Upstream-Fixes: https://bugs.winehq.org/show_bug.cgi?id=47561
Upstream-Fixes: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/967582
Upstream-Fixes: dotnet/coreclr#25071
Upstream-Fixes: dotnet/coreclr#25242
Upstream-Fixes: dotnet/coreclr#25632
Upstream-Fixes: dotnet/coreclr#25709
Upstream-Fixes: dotnet/corefx#37955
Upstream-Fixes: dotnet/corefx#38455
Upstream-Fixes: mono/mono#7377
Upstream-Fixes: mono/mono#8747
Upstream-Fixes: mono/mono#9621
Upstream-Fixes: mono/mono#9664
Upstream-Fixes: mono/mono#9706
Upstream-Fixes: mono/mono#10201
Upstream-Fixes: mono/mono#10645
Upstream-Fixes: mono/mono#10748
Upstream-Fixes: mono/mono#10848
Upstream-Fixes: mono/mono#12141
Upstream-Fixes: mono/mono#13311
Upstream-Fixes: mono/mono#13408
Upstream-Fixes: mono/mono#13412
Upstream-Fixes: mono/mono#13891
Upstream-Fixes: mono/mono#13923
Upstream-Fixes: mono/mono#13945
Upstream-Fixes: mono/mono#14170
Upstream-Fixes: mono/mono#14214
Upstream-Fixes: mono/mono#14214
Upstream-Fixes: mono/mono#14215
Upstream-Fixes: mono/mono#14243
Upstream-Fixes: mono/mono#14377
Upstream-Fixes: mono/mono#14495
Upstream-Fixes: mono/mono#14555
Upstream-Fixes: mono/mono#14724
Upstream-Fixes: mono/mono#14729
Upstream-Fixes: mono/mono#14772
Upstream-Fixes: mono/mono#14792
Upstream-Fixes: mono/mono#14793
Upstream-Fixes: mono/mono#14809
Upstream-Fixes: mono/mono#14830
Upstream-Fixes: mono/mono#14839
Upstream-Fixes: mono/mono#14841
Upstream-Fixes: mono/mono#14847
Upstream-Fixes: mono/mono#14864
Upstream-Fixes: mono/mono#14871
Upstream-Fixes: mono/mono#14917
Upstream-Fixes: mono/mono#14945
Upstream-Fixes: mono/mono#14946
Upstream-Fixes: mono/mono#14948
Upstream-Fixes: mono/mono#14957
Upstream-Fixes: mono/mono#14959
Upstream-Fixes: mono/mono#14960
Upstream-Fixes: mono/mono#14961
Upstream-Fixes: mono/mono#14963
Upstream-Fixes: mono/mono#14971
Upstream-Fixes: mono/mono#14972
Upstream-Fixes: mono/mono#14975
Upstream-Fixes: mono/mono#15023
Upstream-Fixes: mono/mono#15048
Upstream-Fixes: mono/mono#15058
Upstream-Fixes: mono/mono#15080
Upstream-Fixes: mono/mono#15182
Upstream-Fixes: mono/mono#15188
Upstream-Fixes: mono/mono#15189
Upstream-Fixes: mono/mono#15261
Upstream-Fixes: mono/mono#15262
Upstream-Fixes: mono/mono#15263
Upstream-Fixes: mono/mono#15265
Upstream-Fixes: mono/mono#15268
Upstream-Fixes: mono/mono#15307
Upstream-Fixes: mono/mono#15324
Upstream-Fixes: mono/mono#15329
Upstream-Fixes: mono/mono#15330
Upstream-Fixes: mono/mono#15441
Upstream-Fixes: mono/mono#15446
Upstream-Fixes: mono/mono#15503
Upstream-Fixes: mono/mono#15541
Upstream-Fixes: mono/mono#15551
Upstream-Fixes: mono/mono#15556
Upstream-Fixes: mono/mono#15596
Upstream-Fixes: mono/mono#15691
Upstream-Fixes: mono/mono#15692
Upstream-Fixes: mono/mono#15740
Upstream-Fixes: mono/mono#15751
Upstream-Fixes: mono/mono#15760
Upstream-Fixes: mono/mono#15781
Upstream-Fixes: mono/mono#15794
Upstream-Fixes: mono/mono#15825
Upstream-Fixes: mono/mono#15853
Upstream-Fixes: mono/mono#15878
Upstream-Fixes: mono/mono#15887
Upstream-Fixes: mono/mono#15990
Upstream-Fixes: mono/mono#16032
Upstream-Fixes: mono/mono#16411
Upstream-Fixes: mono/mono#16486
Upstream-Fixes: https://github.com/mono/mono/issues/25709
Upstream-Fixes: https://github.com/mono/mono/issues/38821
Upstream-Fixes: #3112
Upstream-Fixes: #3168

Update `build-tools/automation/build.groovy` so that it fully cleans
the build tree before starting the build, so that the vestigial mono
submodule (removed in 0c9f83b) is *actually* removed from the build
tree, so that we don't inadvertently use *ancient* submodule contents.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Sep 6, 2019
Changes: http://github.com/mono/mono/compare/2c3aeaf3780de7392a0d3cbe4dcf86846eb4dffa...29b1ac19c961b959a09097dbc0fe4cd567cc5298

	$ git diff --shortstat 2c3aeaf3..29b1ac19
	 1528 files changed, 45421 insertions(+), 21967 deletions(-)

Changes: mono/api-doc-tools@d03e819...5da8127

	$ git diff --shortstat d03e8198..5da8127a
	 1001 files changed, 86168 insertions(+), 11863 deletions(-)

Changes: mono/api-snapshot@e09042d...1ca8e82

	$ git diff --shortstat e09042da..1ca8e82f
        28 files changed, 612 insertions(+), 217 deletions(-)

Changes: mono/cecil@a6c8f5e...cb6c1ca

	$ git diff --shortstat a6c8f5e1..cb6c1ca9
	 13 files changed, 233 insertions(+), 88 deletions(-)

Changes: mono/corefx@4806207...470e0e1

	$ git diff --shortstat 4806207f...470e0e10
	 4 files changed, 31 insertions(+), 12 deletions(-)

Changes: mono/linker@ebe2a1f...1f87de3

	$ git diff --shortstat ebe2a1f4...1f87de35
	 90 files changed, 3219 insertions(+), 1224 deletions(-)

Changes: xamarin/java.interop@75b1189...4fd3539

	$ git diff --shortstat 75b11891...4fd35393
	 34 files changed, 448 insertions(+), 52 deletions(-)

Upstream-Fixes: https://bugs.winehq.org/show_bug.cgi?id=47561
Upstream-Fixes: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/967582
Upstream-Fixes: dotnet/coreclr#25071
Upstream-Fixes: dotnet/coreclr#25242
Upstream-Fixes: dotnet/coreclr#25632
Upstream-Fixes: dotnet/coreclr#25709
Upstream-Fixes: dotnet/corefx#37955
Upstream-Fixes: dotnet/corefx#38455
Upstream-Fixes: mono/mono#7377
Upstream-Fixes: mono/mono#8747
Upstream-Fixes: mono/mono#9621
Upstream-Fixes: mono/mono#9664
Upstream-Fixes: mono/mono#9706
Upstream-Fixes: mono/mono#10201
Upstream-Fixes: mono/mono#10645
Upstream-Fixes: mono/mono#10748
Upstream-Fixes: mono/mono#10848
Upstream-Fixes: mono/mono#12141
Upstream-Fixes: mono/mono#13311
Upstream-Fixes: mono/mono#13408
Upstream-Fixes: mono/mono#13412
Upstream-Fixes: mono/mono#13891
Upstream-Fixes: mono/mono#13923
Upstream-Fixes: mono/mono#13945
Upstream-Fixes: mono/mono#14170
Upstream-Fixes: mono/mono#14214
Upstream-Fixes: mono/mono#14214
Upstream-Fixes: mono/mono#14215
Upstream-Fixes: mono/mono#14243
Upstream-Fixes: mono/mono#14377
Upstream-Fixes: mono/mono#14495
Upstream-Fixes: mono/mono#14555
Upstream-Fixes: mono/mono#14724
Upstream-Fixes: mono/mono#14729
Upstream-Fixes: mono/mono#14772
Upstream-Fixes: mono/mono#14792
Upstream-Fixes: mono/mono#14793
Upstream-Fixes: mono/mono#14809
Upstream-Fixes: mono/mono#14830
Upstream-Fixes: mono/mono#14839
Upstream-Fixes: mono/mono#14841
Upstream-Fixes: mono/mono#14847
Upstream-Fixes: mono/mono#14864
Upstream-Fixes: mono/mono#14871
Upstream-Fixes: mono/mono#14917
Upstream-Fixes: mono/mono#14945
Upstream-Fixes: mono/mono#14946
Upstream-Fixes: mono/mono#14948
Upstream-Fixes: mono/mono#14957
Upstream-Fixes: mono/mono#14959
Upstream-Fixes: mono/mono#14960
Upstream-Fixes: mono/mono#14961
Upstream-Fixes: mono/mono#14963
Upstream-Fixes: mono/mono#14971
Upstream-Fixes: mono/mono#14972
Upstream-Fixes: mono/mono#14975
Upstream-Fixes: mono/mono#15023
Upstream-Fixes: mono/mono#15048
Upstream-Fixes: mono/mono#15058
Upstream-Fixes: mono/mono#15080
Upstream-Fixes: mono/mono#15182
Upstream-Fixes: mono/mono#15188
Upstream-Fixes: mono/mono#15189
Upstream-Fixes: mono/mono#15261
Upstream-Fixes: mono/mono#15262
Upstream-Fixes: mono/mono#15263
Upstream-Fixes: mono/mono#15265
Upstream-Fixes: mono/mono#15268
Upstream-Fixes: mono/mono#15307
Upstream-Fixes: mono/mono#15324
Upstream-Fixes: mono/mono#15329
Upstream-Fixes: mono/mono#15330
Upstream-Fixes: mono/mono#15441
Upstream-Fixes: mono/mono#15446
Upstream-Fixes: mono/mono#15503
Upstream-Fixes: mono/mono#15541
Upstream-Fixes: mono/mono#15551
Upstream-Fixes: mono/mono#15556
Upstream-Fixes: mono/mono#15596
Upstream-Fixes: mono/mono#15691
Upstream-Fixes: mono/mono#15692
Upstream-Fixes: mono/mono#15740
Upstream-Fixes: mono/mono#15751
Upstream-Fixes: mono/mono#15760
Upstream-Fixes: mono/mono#15781
Upstream-Fixes: mono/mono#15794
Upstream-Fixes: mono/mono#15825
Upstream-Fixes: mono/mono#15853
Upstream-Fixes: mono/mono#15878
Upstream-Fixes: mono/mono#15887
Upstream-Fixes: mono/mono#15990
Upstream-Fixes: mono/mono#16032
Upstream-Fixes: mono/mono#16411
Upstream-Fixes: mono/mono#16486
Upstream-Fixes: https://github.com/mono/mono/issues/25709
Upstream-Fixes: https://github.com/mono/mono/issues/38821
Upstream-Fixes: xamarin#3112
Upstream-Fixes: xamarin#3168

Update `build-tools/automation/build.groovy` so that it fully cleans
the build tree before starting the build, so that the vestigial mono
submodule (removed in 0c9f83b) is *actually* removed from the build
tree, so that we don't inadvertently use *ancient* submodule contents.
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 7, 2019
Changes: http://github.com/mono/mono/compare/2c3aeaf3780de7392a0d3cbe4dcf86846eb4dffa...29b1ac19c961b959a09097dbc0fe4cd567cc5298

	$ git diff --shortstat 2c3aeaf3..29b1ac19
	 1528 files changed, 45421 insertions(+), 21967 deletions(-)

Changes: mono/api-doc-tools@d03e819...5da8127

	$ git diff --shortstat d03e8198..5da8127a
	 1001 files changed, 86168 insertions(+), 11863 deletions(-)

Changes: mono/api-snapshot@e09042d...1ca8e82

	$ git diff --shortstat e09042da..1ca8e82f
        28 files changed, 612 insertions(+), 217 deletions(-)

Changes: mono/cecil@a6c8f5e...cb6c1ca

	$ git diff --shortstat a6c8f5e1..cb6c1ca9
	 13 files changed, 233 insertions(+), 88 deletions(-)

Changes: mono/corefx@4806207...470e0e1

	$ git diff --shortstat 4806207f...470e0e10
	 4 files changed, 31 insertions(+), 12 deletions(-)

Changes: mono/linker@ebe2a1f...1f87de3

	$ git diff --shortstat ebe2a1f4...1f87de35
	 90 files changed, 3219 insertions(+), 1224 deletions(-)

Changes: xamarin/java.interop@75b1189...4fd3539

	$ git diff --shortstat 75b11891...4fd35393
	 34 files changed, 448 insertions(+), 52 deletions(-)

Fixes: #3112
Fixes: #3168

Context: https://bugs.winehq.org/show_bug.cgi?id=47561
Context: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/967582
Context: dotnet/coreclr#25071
Context: dotnet/coreclr#25242
Context: dotnet/coreclr#25632
Context: dotnet/coreclr#25709
Context: dotnet/corefx#37955
Context: dotnet/corefx#38455
Context: mono/mono#7377
Context: mono/mono#8747
Context: mono/mono#9621
Context: mono/mono#9664
Context: mono/mono#9706
Context: mono/mono#10201
Context: mono/mono#10645
Context: mono/mono#10748
Context: mono/mono#10848
Context: mono/mono#12141
Context: mono/mono#13311
Context: mono/mono#13408
Context: mono/mono#13412
Context: mono/mono#13891
Context: mono/mono#13923
Context: mono/mono#13945
Context: mono/mono#14170
Context: mono/mono#14214
Context: mono/mono#14214
Context: mono/mono#14215
Context: mono/mono#14243
Context: mono/mono#14377
Context: mono/mono#14495
Context: mono/mono#14555
Context: mono/mono#14724
Context: mono/mono#14729
Context: mono/mono#14772
Context: mono/mono#14792
Context: mono/mono#14793
Context: mono/mono#14809
Context: mono/mono#14830
Context: mono/mono#14839
Context: mono/mono#14841
Context: mono/mono#14847
Context: mono/mono#14864
Context: mono/mono#14871
Context: mono/mono#14917
Context: mono/mono#14945
Context: mono/mono#14946
Context: mono/mono#14948
Context: mono/mono#14957
Context: mono/mono#14959
Context: mono/mono#14960
Context: mono/mono#14961
Context: mono/mono#14963
Context: mono/mono#14971
Context: mono/mono#14972
Context: mono/mono#14975
Context: mono/mono#15023
Context: mono/mono#15048
Context: mono/mono#15058
Context: mono/mono#15080
Context: mono/mono#15182
Context: mono/mono#15188
Context: mono/mono#15189
Context: mono/mono#15261
Context: mono/mono#15262
Context: mono/mono#15263
Context: mono/mono#15265
Context: mono/mono#15268
Context: mono/mono#15307
Context: mono/mono#15324
Context: mono/mono#15329
Context: mono/mono#15330
Context: mono/mono#15441
Context: mono/mono#15446
Context: mono/mono#15503
Context: mono/mono#15541
Context: mono/mono#15551
Context: mono/mono#15556
Context: mono/mono#15596
Context: mono/mono#15691
Context: mono/mono#15692
Context: mono/mono#15740
Context: mono/mono#15751
Context: mono/mono#15760
Context: mono/mono#15781
Context: mono/mono#15794
Context: mono/mono#15825
Context: mono/mono#15853
Context: mono/mono#15878
Context: mono/mono#15887
Context: mono/mono#15990
Context: mono/mono#16032
Context: mono/mono#16411
Context: mono/mono#16486
Context: https://github.com/mono/mono/issues/25709
Context: https://github.com/mono/mono/issues/38821

Update `build-tools/automation/build.groovy` so that it fully cleans
the build tree before starting the build, so that the vestigial mono
submodule (removed in 0c9f83b) is *actually* removed from the build
tree, so that we don't inadvertently use *ancient* submodule contents.
BrzVlad added a commit to BrzVlad/mono that referenced this issue Sep 24, 2019
When we want to close the connection, setting the close flag is not enough, because a thread can be already stuck in a blocking receive, without getting to check the flag. We fix this by additionaly breaking the connection, by using socket.Shutdown.

Fixes mono#7377
@srxqds

This comment has been minimized.

Copy link

@srxqds srxqds commented Sep 25, 2019

For a Mono process with the soft debugger on, running on port 4567:

static void Main(string[] args)
{
	var vm = VirtualMachineManager.Connect(new IPEndPoint(IPAddress.Loopback, 4567));
	vm.Detach();

	vm = VirtualMachineManager.Connect(new IPEndPoint(IPAddress.Loopback, 4567));
	vm.Detach();

	Console.ReadLine();
}

We're seeing two threads that are still running and waiting:

threads

Both threads are waiting on Receive with this stacktrace:

Not Flagged		21928	6	Worker Thread	SDB Receiver

System.dll!System.Net.Sockets.Socket.Receive
[Managed to Native Transition]	 
System.dll!System.Net.Sockets.Socket.Receive(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.TcpConnection.TransportReceive(byte[] buf, int buf_offset, int len)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.Receive(byte[] buf, int buf_offset, int len)	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.ReadPacket()	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.ReceivePacket()	 
Mono.Debugger.Soft.dll!Mono.Debugger.Soft.Connection.receiver_thread_main()	 
mscorlib.dll!System.Threading.ThreadHelper.ThreadStart_Context(object state)	 
mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	 
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	 
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)	 
mscorlib.dll!System.Threading.ThreadHelper.ThreadStart()	 

The way I look at it in VirtualMachine.Detach

public void Detach () {
	conn.VM_Dispose ();
	conn.Close ();
	notify_vm_event (EventType.VMDisconnect, SuspendPolicy.None, 0, 0, null, 0);
}

VM_Dispose tells the debuggee that we're disposing, and then Connection.Close sets the closed flag to true. Nothing interrupts the receiver_thread_main loop which is waiting.

We've fixed it by switching the calls:

conn.Close ()
conn.VM_Dispose()

This way we're setting the closed flag to true, sending and reading the reply to VM_Dispose, and exiting the receiver_thread_main.

@vargaz, what do you think?

hi, how can you debugger mono with visual studio on windows?
can you help me?

@thaystg

This comment has been minimized.

Copy link
Contributor

@thaystg thaystg commented Sep 25, 2019

Hi, @srxqds , why do you want to do this?

@srxqds

This comment has been minimized.

Copy link

@srxqds srxqds commented Sep 26, 2019

Hi, @srxqds , why do you want to do this?

I want make vs debugger we embeded mono. But I don't know is there already a vs debugger asix plugin.

BrzVlad added a commit to BrzVlad/mono that referenced this issue Sep 26, 2019
When we want to close the connection, setting the close flag is not enough, because a thread can be already stuck in a blocking receive, without getting to check the flag. We fix this by additionaly breaking the connection, by using socket.Shutdown.

Fixes mono#7377
akoeplinger added a commit that referenced this issue Sep 26, 2019
* Revert "[Mono.Debugger.Soft] Properly close connections"

This reverts commit 2da7707.

This was previously reverted because it was causing issues and it still does. It is a hack fix. We signal the connection for closing and next we send the dispose command over the network. This is obviously racy.

* [Mono.Debugger.Soft] Make sure receiving thread always closes

When we want to close the connection, setting the close flag is not enough, because a thread can be already stuck in a blocking receive, without getting to check the flag. We fix this by additionaly breaking the connection, by using socket.Shutdown.

Fixes #7377

* Bump API snapshot submodule
monojenkins added a commit to monojenkins/mono that referenced this issue Sep 26, 2019
When we want to close the connection, setting the close flag is not enough, because a thread can be already stuck in a blocking receive, without getting to check the flag. We fix this by additionaly breaking the connection, by using socket.Shutdown.

Fixes mono#7377
@joj

This comment has been minimized.

Copy link
Member

@joj joj commented Sep 26, 2019

@srxqds depending on embedded to what, the original developer might have included debugger support. If this is something you're developing, we do have this: https://github.com/xamarin/vs-mono-debugger-sample as a sample on how to integrate debugger support into your project system. The sample was developed by us and originally used by some of our teams, but it might be slightly outdated now.

BrzVlad added a commit that referenced this issue Oct 6, 2019
* Revert "[Mono.Debugger.Soft] Properly close connections"

This reverts commit 2da7707.

This was previously reverted because it was causing issues and it still does. It is a hack fix. We signal the connection for closing and next we send the dispose command over the network. This is obviously racy.

* [Mono.Debugger.Soft] Make sure receiving thread always closes

When we want to close the connection, setting the close flag is not enough, because a thread can be already stuck in a blocking receive, without getting to check the flag. We fix this by additionaly breaking the connection, by using socket.Shutdown.

Fixes #7377

* Bump API snapshot submodule
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Oct 10, 2019
Changes: mono/mono@5281037...7dbad3c

Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Oct 11, 2019
Changes: mono/mono@5281037...df5e13f

Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@df5e13f: [tests] Bump corefx to get Azure testhost change (#17275)
  * mono/mono@11e1499: [2019-08] [merp] Print missing status marker file for stage 1 (setup) (#17220)
  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Oct 16, 2019
Changes: mono/mono@5281037...df5e13f

Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@df5e13f: [tests] Bump corefx to get Azure testhost change (#17275)
  * mono/mono@11e1499: [2019-08] [merp] Print missing status marker file for stage 1 (setup) (#17220)
  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Oct 16, 2019
Changes: mono/mono@5281037...3eb5f34

Fixes: xamarin#3726

Context: mono/mono@0f241c9
Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@3eb5f34: [GTK] Bump bockbuild for GtkViewport autoscrolling patch. (#17321)
  * mono/mono@b601371: Update MERP event type to MonoAppCrash
  * mono/mono@6184ff0: [2019-08][ci] Use Xcode11.1 and 11.2beta2 for XI/XM Mono SDK builds (#17324)
  * mono/mono@8969f2c: [2019-08] [merp] Include any managed methods in the 'unmanaged_frames' portion (#17316)
  * mono/mono@3009440: [2019-08][merp] Don't install SIGTERM handler in EnableMicrosoftTelemetry (#17308)
  * mono/mono@df5e13f: [tests] Bump corefx to get Azure testhost change (#17275)
  * mono/mono@11e1499: [2019-08] [merp] Print missing status marker file for stage 1 (setup) (#17220)
  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Oct 16, 2019
Changes: https://github.com/mono/mono/compare/528103728fc2aedb7b6062e11255d39a0ed3f31c... df5e13f95df7a2d11d86904e74b1bd8950c9d43b

Fixes: #3726

Context: mono/mono@0f241c9
Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@df5e13f: [tests] Bump corefx to get Azure testhost change (#17275)
  * mono/mono@11e1499: [2019-08] [merp] Print missing status marker file for stage 1 (setup) (#17220)
  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Oct 17, 2019
Changes: mono/mono@5281037...3eb5f34

Fixes: #3726

Context: mono/mono@0f241c9
Context: mono/mono#7377
Context: mono/mono#16570
Context: mono/mono#17004
Context: mono/mono#17151
Context: mono/mono#17180

  * mono/mono@3eb5f34: [GTK] Bump bockbuild for GtkViewport autoscrolling patch. (#17321)
  * mono/mono@b601371: Update MERP event type to MonoAppCrash
  * mono/mono@6184ff0: [2019-08][ci] Use Xcode11.1 and 11.2beta2 for XI/XM Mono SDK builds (#17324)
  * mono/mono@8969f2c: [2019-08] [merp] Include any managed methods in the 'unmanaged_frames' portion (#17316)
  * mono/mono@3009440: [2019-08][merp] Don't install SIGTERM handler in EnableMicrosoftTelemetry (#17308)
  * mono/mono@df5e13f: [tests] Bump corefx to get Azure testhost change (#17275)
  * mono/mono@11e1499: [2019-08] [merp] Print missing status marker file for stage 1 (setup) (#17220)
  * mono/mono@7dbad3c: [arm] Fix fetching of method addresses (#17253)
  * mono/mono@9a88a36: [sgen] Fix invalid value passed to write barrier (#17247)
  * mono/mono@0f241c9: [2019-08] Add drawing type converters to mobile profiles (#17240)
  * mono/mono@7ebe1a1: Update Roslyn to 3.4.0-beta2-19477-01
  * mono/mono@b759449: Bump msbuild to track mono-2019-08
  * mono/mono@617f399: [IO] Remove read-only logic in mono_w32_get_disk_free_space (#17211)
  * mono/mono@77258ea: [2019-08] [debugger][exception] Debugger breaks on handled exceptions (#17202)
  * mono/mono@f83c321: Bump msbuild to track mono-2019-08 (#17193)
  * mono/mono@1ecd094: [2019-08] [Mono.Debugger.Soft] Fix VirtualMachine detaching (#17077)
  * mono/mono@54a33be: [merp] Put thread into async context before running summarizer (#17197)
  * mono/mono@72128bb: Bump libgdiplus to 6.0.4
  * mono/mono@65a972c: Always do copy_stack_data on entering GC safe/unsafe mode. (#17184)
  * mono/mono@9e6def1: [merp] exit_status is 0 if we ran the uploader successfully (#17187)
  * mono/mono@8a707cc: [2019-08] [reflection] Only duplicate MonoMarshalSpec strings for custom types (#17189)
  * mono/mono@bd72952: [2019-08] [merp] Don't overrun buffer in copy_summary_string_safe … (#17178)
  * mono/mono@b6efc0c: Bump msbuild to track xplat-master (#17132)
  * mono/mono@2869cd5: Bump ikvm to get mono/ikvm-fork#13 (#17170)
  * mono/mono@a64a256: [2019-08] [merp] Use macOS version not Darwin version in MERP reports (#17147)
  * mono/mono@57f0684: [2019-08] [merp] Add API method that whitelists all native libraries (#17128)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.