Bump mono to head of 2017-12 and include profiler fix #4135

spouliot · 2018-05-28T12:49:55Z

references:
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/599395
mono/mono#7683

Commit list for mono/mono:

mono/mono@4d38f695074 [profiler] Switch tls storage to use mono_thread_info_get/set_tools_data. Fixes [Device tests] Bump maccore to fix device tests. #7683
mono/mono@7344e176204 [mono-threads] Introduce mono_thread_info_get/set_tools_data so they can store data sanely.

Diff: mono/mono@ea8a24b...4d38f69

references: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/599395 mono/mono#7683 Commit list for mono/mono: * mono/mono@4d38f695074 [profiler] Switch tls storage to use mono_thread_info_get/set_tools_data. Fixes dotnet#7683 * mono/mono@7344e176204 [mono-threads] Introduce mono_thread_info_get/set_tools_data so they can store data sanely. Diff: mono/mono@ea8a24b...4d38f69

cosminstirbu · 2018-05-28T15:22:31Z

Hi,

I know this is still pending some checks, but I was wondering, once this is merged what would be a good way to check if this fixes the related crash without waiting for a beta / prod release?

Is there a way to link / download a nightly / alpha build and install it in VS for Mac?

We're currently developing two iOS apps and we're struggling chasing down memory leaks due to this crash so it would be extremely helpful to integrate this fix as soon as we can and see if it fixes our issue.

Thank you,
Cosmin

monojenkins · 2018-05-28T15:55:41Z

Build success
Build comment file:

✅ Provisioning succeeded
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only)
✅ Generator Diff
✅ Test run succeeded

spouliot · 2018-05-28T17:54:43Z

@cosminstirbu yes, all our packages (from builds) are public.

So, once merged, you'll be able to download and install it locally and we would appreciate confirmation/feedback. Thanks :)

cosminstirbu · 2018-05-31T08:19:38Z

Sorry if this might annoy anyone but I'm really interesting in seeing this approved and merged so I can test the fix.

Memory Profiling is a big think for us, especially that we're building an enterprise application that will be used 8h / day.

Thank you,
Cosmin

spouliot · 2018-05-31T12:47:22Z

@cosminstirbu the release of Xcode 9.4 pushed back the testing of this fix

However the same fix is also available from master (which is not frozen like the release branch), if you want to test it the build can also be downloaded from https://jenkins.mono-project.com/view/Xamarin.MaciOS/job/xamarin-macios-builds-master/

cosminstirbu · 2018-05-31T13:53:33Z

I've picked up

Xamarin.Mac
Version: 4.5.0.337 (Visual Studio Enterprise)

Xamarin.iOS
Version: 11.11.0.337 (Visual Studio Enterprise)
Hash: c993da4
Branch: master
Build date: 2018-05-31 05:05:28-0400

And I receive the same error :(

error: * Assertion at /Users/builder/data/lanes/1381/c993da4c/source/xamarin-macios/external/mono/mono/profiler/log.c:557, condition `mono_lls_insert (&log_profiler.profiler_thread_list, hp, &thread->node) && "Why can't we insert the thread in the LLS?"' not met

cosminstirbu · 2018-05-31T13:56:21Z

However I'm not sure it includes the fix, since if I look at https://github.com/mono/mono/pull/8824/files#diff-ec568e06073848ecf046f94dba5826c0

line 557 in log.c doesn't really include any assertion, so either the line doesn't match or I'm missing something.

cosminstirbu · 2018-05-31T14:07:15Z

This is the thread that crashed:

Thread 10 name:  tid_a803
Thread 10 Crashed:
0   libsystem_c.dylib             	0x1a6b69ea __abort + 132
1   libsystem_c.dylib             	0x1a6b69ea __abort + 132
2   libsystem_c.dylib             	0x1a6b6966 __abort + 0
3   ...MyApp.iOS	0x038a0ff8 log_callback(char const*, char const*, char const*, int, void*) + 58863608 (runtime.m:1212)
4   ...MyApp.iOS	0x03855ca0 eglib_log_adapter + 58555552 (mono-logger.c:395)
5   ...MyApp.iOS	0x0386413c monoeg_g_logv + 58614076 (goutput.c:116)
6   ...MyApp.iOS	0x0386426c monoeg_assertion_message + 58614380 (goutput.c:148)
7   ...MyApp.iOS	0x038c2812 emit_method + 59000850 (log.c:878)
8   ...MyApp.iOS	0x038c08ea gc_root_deregister + 58992874 (log.c:1278)
9   ...MyApp.iOS	0x037e2f50 mono_profiler_raise_gc_root_unregister + 58085200 (profiler-events.h:97)
10  ...MyApp.iOS	0x037fd7c2 mono_thread_detach_internal + 58193858 (threads.c:898)
11  ...MyApp.iOS	0x0380067c thread_detach + 58205820 (threads.c:3072)
12  ...MyApp.iOS	0x0385b264 unregister_thread + 58577508 (mono-threads.c:481)
13  ...MyApp.iOS	0x0385b870 thread_info_key_dtor + 58579056 (mono-threads.c:740)
14  libsystem_pthread.dylib       	0x1a7d741a _pthread_tsd_cleanup + 404
15  libsystem_pthread.dylib       	0x1a7d71f6 _pthread_exit + 158
16  libsystem_pthread.dylib       	0x1a7d6984 pthread_mutex_lock + 0
17  libsystem_pthread.dylib       	0x1a7d64cc start_wqthread + 8

spouliot · 2018-05-31T14:55:30Z

@cosminstirbu 337 is older than merge that include the fix. Please try the latest build available
https://bosstoragemirror.azureedge.net/wrench/macios-mac-master/30/30d644d5a11b3dd4a46c9ca9c28e22259f452a65/xamarin.ios-11.11.0.368.pkg

cosminstirbu · 2018-05-31T15:25:55Z

My initial testing indicates that the issue was fixed :D.

We'll do some more testing early next week, but so far things look good.

Thank you!

GouriKumari · 2018-05-31T21:32:08Z

I am unable to reproduce the initial issue with current stable XI (11.12.0.4). I also checked with XI (11.8.0.20) and mono5.8 with which the issue was initially reported. In both cases, profiler got started and I could successfully profile the app on both simulator and device. Sample apps used for testing are https://github.com/MvvmCross/MvvmCross-Samples/tree/master/StarWarsSample.
Is there any other way to reproduce this issue?

p-lad · 2018-06-06T06:40:59Z

Hello,
Profiling was successful on an iOS application. Tried it with the Star Wars sample as well. So, as discussed on slack with @rodrmoya , I took the latest builds for XI and as the issue is not reproduced; marking it as verified. The issue was not reproduced when tested on:

Build info:

Visual Studio Enterprise 2017 for Mac (Preview)Version 7.6 Preview (7.6 build 1509)

Mono 5.12.0.260 (2018-02/58637d0ee7c) (64-bit)

Xamarin.Mac Version: 4.5.0.373 (Visual Studio Enterprise)

Xamarin.iOS Version: 11.13.0.0 (Visual Studio Enterprise)

Build gist:
https://gist.github.com/parasAmbhore/519bc17aded2c80eb7227812537db1fe

Xamarin iOS Version taken from:
https://github.com/xamarin/xamarin-macios/commits/d15-8

Mono Version taken:
https://xamjenkinsartifact.azureedge.net/build-package-osx-mono/2018-02/208/58637d0ee7c4848210eb6c2df49ee4f35b70f7ab/MonoFramework-MDK-5.12.0.260.macos10.xamarin.universal.pkg

Visual Studio:
https://wrench.internalx.com/Wrench/ViewLane.aspx?lane_id=6089&host_id=460&revision_id=938726

Star Wars Sample taken from:
https://github.com/MvvmCross/MvvmCross-Samples

Screencast link:
https://www.screencast.com/t/86vDVteg

Thanks

spouliot · 2018-06-14T17:52:30Z

This change did not meet the minimum criteria for a 15.7 service release.
The workaround is to use the latest 15.8 preview to profile applications that hits this particular assertion.

spouliot added the requires-approval-before-merge The pull request requires special approval before it can be merged label May 28, 2018

spouliot added this to the d15-7 milestone May 28, 2018

rolfbjarne approved these changes May 28, 2018

View reviewed changes

dalexsoto approved these changes May 28, 2018

View reviewed changes

spouliot closed this Jun 14, 2018

Bump mono to head of 2017-12 and include profiler fix #4135

Bump mono to head of 2017-12 and include profiler fix #4135

Uh oh!

Conversation

spouliot commented May 28, 2018

Uh oh!

cosminstirbu commented May 28, 2018

Uh oh!

monojenkins commented May 28, 2018

Build success Build comment file:

Uh oh!

spouliot commented May 28, 2018

Uh oh!

cosminstirbu commented May 31, 2018

Uh oh!

spouliot commented May 31, 2018

Uh oh!

cosminstirbu commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cosminstirbu commented May 31, 2018

Uh oh!

cosminstirbu commented May 31, 2018

Uh oh!

spouliot commented May 31, 2018

Uh oh!

cosminstirbu commented May 31, 2018

Uh oh!

GouriKumari commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-lad commented Jun 6, 2018

Uh oh!

spouliot commented Jun 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Build success
Build comment file:

cosminstirbu commented May 31, 2018 •

edited

Loading

GouriKumari commented May 31, 2018 •

edited

Loading