Bug 52698 - Event handler with capturing lambda not hooked up
Summary: Event handler with capturing lambda not hooked up
Status: CONFIRMED
Alias: None
Product: iOS
Classification: Xamarin
Component: General (show other bugs)
Version: master
Hardware: PC Windows
: Normal normal
Target Milestone: Future Cycle (TBD)
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-02-23 05:54 UTC by Kent
Modified: 2017-03-06 00:06 UTC (History)
7 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Repro solution (165.83 KB, application/x-zip-compressed)
2017-02-23 05:54 UTC, Kent
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 52698 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
CONFIRMED

Description Kent 2017-02-23 05:54:10 UTC
Created attachment 19918 [details]
Repro solution

I apologize if I didn't get the categorization of this bug correct. I'm quite sure it's a compiler/runtime problem, but wasn't sure exactly where to stick it.

Consider the following:

    public static void Hook(this INotifyPropertyChanged @this)
    {
        var somethingToCapure = "hello";

        @this.PropertyChanged += (s, e) =>
        {
            var dummy = 42;
        };

        @this.PropertyChanged += (s, e) =>
        {
            var dummy = 42;
            var captured = somethingToCapure;
        };

        PropertyChangedEventHandler handler1 = (s, e) =>
        {
            var dummy = 42;
        };
        @this.PropertyChanged += handler1;

        PropertyChangedEventHandler handler2 = (s, e) =>
        {
            var dummy = 42;
            var captured = somethingToCapure;
        };
        @this.PropertyChanged += handler2;
    }

When hooking a component that implements INotifyPropertyChanged directly, all four handlers are executed as expected. However, when hooking a component that implements INotifyPropertyChanged indirectly through ReactiveUI, only the non-capturing lambdas are executed.

This is a regression, but I can't tell you exactly when this occurred. We have code in production that works fine, and it was whilst trying to re-use said code in a new project that I noticed it no longer works.

I've attached a repro. Have tried both Cycle 8 and 9 (via a colleague) and the result was the same.
Comment 1 Zoltan Varga 2017-02-23 20:19:44 UTC
I can also reproduce this with cycle6, i.e.:

Xamarin.iOS
Version: 9.8.2.22 (Xamarin Enterprise)
Hash: f37444a
Branch: cycle7-sr1
Build date: 2016-07-28 12:17:02-0400

Runtime:
	Mono 4.6.2 (mono-4.6.0-branch/ac9e222) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 406020016
Comment 2 Vincent Dondain [MSFT] 2017-02-27 06:14:41 UTC
I could also reproduce this with the following environment: https://gist.github.com/VincentDondain/3a32f2ec661c9d8a3d3d0dc37749410b
Comment 3 Zoltan Varga 2017-02-27 06:19:08 UTC
Since this is also reproducible with cycle6, what version is this worked ?
Comment 4 Kent 2017-02-27 06:22:45 UTC
I believe it was just an earlier version of cycle 8. The last production build we have is dated Jan 4, 2017. I'm guessing whatever cycle 8 was out at the time should work. If not, I'm at a loss.
Comment 5 Kent 2017-03-06 00:06:55 UTC
Sorry, forget everything I said about this working in a prior build. I had written the code in a previous project, unit tested it (tests run against full framework just fine), but I never actually used the code in the product. I must have concocted an alternative solution, probably because I couldn't get it to work!

Really happy to see this confirmed and at least targeted for a future cycle. I have a feeling it's also root cause of other insidious problems, such as that reported by #31415.