Giter Club home page Giter Club logo

Comments (15)

ulrichb avatar ulrichb commented on August 17, 2024 1

Note that also on pre .NET 7 ProxyGenerator wasn't be able to intercept methods with Spans.

The following program throws an "InvalidProgramException: Cannot create boxed ByRef-like values." on .NET 3.1, 5 and 6 ... and "InvalidProgramException: Common Language Runtime detected an invalid program." on .NET 7 (actually the old exception message was better 🤦).

var proxyGenerator = new ProxyGenerator();

using var stream = new MemoryStream(Encoding.UTF8.GetBytes("some text"));

var proxiedStream = proxyGenerator.CreateClassProxyWithTarget(stream, new MyInterceptor());

Span<byte> spanBuffer = stackalloc byte[20];
proxiedStream.Read(spanBuffer);

Console.WriteLine(Encoding.UTF8.GetString(spanBuffer));

class MyInterceptor : IInterceptor
{
    public void Intercept(IInvocation invocation) => invocation.Proceed();
}

I think the reason why @drauch 's repro worked in pre .NET 7 is that they probably switched from the byte-array overload to the Span<byte> overload of Stream.Read() at the call site (within XmlReader) in .NET 7.

from core.

ulrichb avatar ulrichb commented on August 17, 2024 1

Proposal: A new ProxyGenerationOptions.SignaturesWithSpans enum setting with the following options:

  • SignaturesWithSpans.Throw (default): Validates at proxy construction time that there is no method including a Span. (Better than the current behavior at method invocation time). Note that this is a breaking change.
  • SignaturesWithSpans.ThrowOnInvocation: Only for compatibility with the old behavior.
  • SignaturesWithSpans.ReplaceWithNull: As @stakx suggested above.
  • SignaturesWithSpans.BoxIntoArray: Copies the span into an array (in case the user needs the value and accepts the perf hit and mem allocation).

The last option is the most expensive one and therefore could be added later.

from core.

drauch avatar drauch commented on August 17, 2024

Note: I just checked, even without any interceptors the program fails.

from core.

drauch avatar drauch commented on August 17, 2024

Here is the full repro VS 2022 solution: https://www.dropbox.com/s/4evu8rm636jv8h8/ReproInvalidProgramExceptionXmlReader.zip?dl=0

from core.

stakx avatar stakx commented on August 17, 2024

I haven't looked at the repro solution yet, but I suspect this will come down to .NET 7's MemoryStream having some method with a Span<> parameter (possibly used in combination with the C# in keyword). Span<> is a by-ref-like type, and those aren't (and in all likelihood cannot be) supported by DynamicProxy. The reason is that we have no way of transferring such arguments into the invocation's object[]-typed Arguments array (since that would mean that they can escape from the evaluation stack to longer-lived heap memory, thereby violating up the lifetime guarantees C# gives you for them).

from core.

jonorossi avatar jonorossi commented on August 17, 2024

I don't think we've got any tests for Span, however we do for a ref struct. DP might still be missing something to ensure it's valid, however there are a lot of limitations with references because parameters need to go into the invocation object.

In general, I think proxying a MemoryStream just sounds like a bad idea.

from core.

jonorossi avatar jonorossi commented on August 17, 2024

@stakx we did it again. Only saw your comment as I clicked the green button 😆

from core.

ulrichb avatar ulrichb commented on August 17, 2024

... yep:

image

from core.

drauch avatar drauch commented on August 17, 2024

@ulrichb Ah, thanks, that makes it clear why it fails in .NET 7 and didn't fail before.

@stakx @jonorossi So what you're saying is that we cannot use a proxy for a Stream at all basically?

It'd be unfortunate, because we have a "whatever you call on this stream instance, if an XyzException is thrown, translate it to AbcException"-necessity (in reality it's not a MemoryStream, I just used the MemoryStream for the repro sample). Our only other option is basically to write a decorator and decorate each method manually and make sure via a test that we really have all methods decorated :-/

Best regards,
D.R.

from core.

stakx avatar stakx commented on August 17, 2024

I'll take a look at our test suite. Right now it would appear that code generation succeeds despite the invalid attempt to box a Span<> (implying PEVerify doesn't catch that error).

I don't see any way to support spans, at this time I can think of a few workarounds / mitigations.

  • DynamicProxy could skip problematic arguments by simply substituting null in the invocation argument array.

  • We could perhaps add a configurable marshalling layer that translates between problematic types (like Span<> and types that can be safely boxed. Not sure how exactly that would work and whether it's worth the effort at this time.

from core.

stakx avatar stakx commented on August 17, 2024

what you're saying is that we cannot use a proxy for a Stream at all basically?

@drauch, I still haven't looked at the repro code, nor have I recently tried to create a Stream proxy object. The answer to your quesrion depends on when exactly the error occurs:

  • During proxy type generation: In that case yes, you probably cannot proxy streams with DynamicProxy.

  • During single calls to the generated proxy: In that case you can proxy streams, but some methods will be unusable.

from core.

drauch avatar drauch commented on August 17, 2024

It's a bit unfortunate, because I don't even touch the parameters in my IInterceptor, they only need to be passed on to the wrapped Stream. But I guess it's hard to integrate that in the existing API.

Thank you for you quick replies.

from core.

stakx avatar stakx commented on August 17, 2024

@ulrichb, understood. Note that ideally, any such new option should cover all problematic (by-ref-like?) types, not just Span<> (even though that is probably the most prominent example).

from core.

markusschaber avatar markusschaber commented on August 17, 2024

@ulrichb, understood. Note that ideally, any such new option should cover all problematic (by-ref-like?) types, not just Span<> (even though that is probably the most prominent example).

I agree that Span<> and ReadOnlySpan<> are probably the most common ref structs, and they're getting more and more common within the framework, so I think a workaround for those two would fix 90% of all cases...

(I came here as a FakeItEasy user, which uses castle under the hood.)

from core.

stakx avatar stakx commented on August 17, 2024

I've opened a new issue to discuss and plan support for by-ref-like types; see #663.

from core.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.