protobuf-net / protobuf-net.Grpc

GRPC bindings for protobuf-net and grpc-dotnet
Other
846 stars 106 forks source link

Error starting gRPC call. HttpRequestException: An error occurred while sending the request. InvalidOperationException: No serializer defined for type: System.IntPtr #282

Open shivkumarghatbale opened 1 year ago

shivkumarghatbale commented 1 year ago

Data contract looks like below, it has one IntPtr data member. How do I handle this case

public class setCommCellIdRequest {

[ProtoMember(1)] public Int64 commCellId { get; set; }

[ProtoMember(2)] public IntPtr m_pEvAlertObj { get; set; }

[ProtoMember(3)] public Boolean isDisposed { get; set; } }

@mgravell

mgravell commented 1 year ago

What do we expect to happen here? Right now this is intentionally not supported, but if it was: what would it mean to you?

  1. send a raw number that has no meaning / significance except that which the receiver imparts
  2. send a nul-terminated buffer starting at that address
  3. send a length-based buffer starting at that address (with length ... from where?)
  4. do something else?
shivkumarghatbale commented 1 year ago

@mgravell thanks for quick check. I think option #1 is suitable.

mgravell commented 1 year ago

OK - so we're really thinking nint more than IntPtr (semantic wise, I mean - they're the same thing at the implementation level). The problem here is: what do we do in some theoretical future world where IntPtr is 128-bit, since varint encoding only handles 64-bit values? Is there any reason not to just use long/ulong here, in your model?

shivkumarghatbale commented 1 year ago

Here I am migrating existing code to use grpc. The existing code deals with .Net code & managed cpp code. In the existing .NET code we've lot of functions which use IntPtr provided by .NET. So using long/ulong takes lot of back and forth process if I choose to use it instead of IntPtr. For brand new projects it is easy to get away from IntPtr.

option #1 or #2 however you wish to implement, Is it possible for you to provide update which supports the serialization of IntPtr. Btw, today max size IntPtr can have is 64 bit, let's implement it for 64 bit. And throw some error for future world 128-bit :-) PS: Great fan of yours sir.

mgravell commented 1 year ago

And of course what to do if a 32-bit process gets a 64-bit value - throw?

mgravell commented 1 year ago

This isn't technically hard - will try to do over the next few days.

shivkumarghatbale commented 1 year ago

We're in a hurry to implement this. If publishing fix version is going to take more than a day then I have to go for converting intptr in long and other way around at server. If it's easy fix, I hope we'll have fix soon

mgravell commented 1 year ago

I can show you how to write a custom scalar serializer. About 8 lines, but I'm in a coffee shop without a computer that the moment :)

Something like (again, not at a computer)

public sealed class Int64Serializer : ISerializer<IntPtr>
            {
                public SerializerFeatures Features
                    => SerializerFeatures.CategoryScalar | SerializerFeatures.WireTypeVarint;

                public IntPtr Read(ref ProtoReader.State state, IntPtr value)
                    => new IntPtr(state.ReadInt64());

                public void Write(ref ProtoWriter.State state, IntPtr value)
                    => state.WriteInt64(value.ToInt64());
            }
...
RuntimeTypeModel.Default.Add<IntPtr>(false).SetSerializer(new Int64Serializer());

This would be wire-compatible with an inbuilt solution.

shivkumarghatbale commented 1 year ago

Would the above solution work (not familiar with custom scalar serializer, will check on this) without having to update protobuf-net.Grpc?

Btw, enjoy your coffee, I bought you a coffee earlier today.

mgravell commented 1 year ago

You'll need to make sure you have a ref to protobuf-net V3, not protobuf-net v2 - the custom serializer API is a V3 feature. This is separate to protobuf-net.Grpc; protobuf-net.Grpc can use either V2 or V3, and exploits V3 features when detected.

mgravell commented 1 year ago

Also, thanks for the coffee! Very timely :)

shivkumarghatbale commented 1 year ago

@mgravell looks like SetSerializer() is not valid method name in "MetaType".

image

shivkumarghatbale commented 1 year ago

Modified above code like this to add serializer. => var mt = RuntimeTypeModel.Default.Add(typeof(IntPtr), true); mt.IncludeSerializerMethod = true; mt.SerializerType = typeof(Int64Serializer); ... public sealed class Int64Serializer : ISerializer { public SerializerFeatures Features => SerializerFeatures.CategoryScalar | SerializerFeatures.WireTypeVarint;

            public IntPtr Read(ref ProtoReader.State state, IntPtr value)
                => new IntPtr(state.ReadInt64());

            public void Write(ref ProtoWriter.State state, IntPtr value)
                => state.WriteInt64(value.ToInt64());
        }

But at runtime I see following error =>

Grpc.Core.RpcException: Status(StatusCode="Unknown", Detail="Exception was thrown by handler. InvalidOperationException: No serializer defined for type: System.IntPtr") at ProtoBuf.Grpc.Internal.Reshape.UnaryTaskAsyncImpl[TRequest,TResponse](AsyncUnaryCall`1 call, MetadataContext metadata, CancellationToken cancellationToken) in /_/src/protobuf-net.Grpc/Internal/Reshape.cs:line 549

shivkumarghatbale commented 1 year ago

Modified the adding of serializer code like below =>

public sealed class Int64Serializer : ISerializer { public SerializerFeatures Features => SerializerFeatures.CategoryScalar | SerializerFeatures.WireTypeVarint;

        public IntPtr Read(ref ProtoReader.State state, IntPtr value)
            => new IntPtr(state.ReadInt64());

        public void Write(ref ProtoWriter.State state, IntPtr value)
            => state.WriteInt64(value.ToInt64());
    }

... var model = RuntimeTypeModel.Create(); model.AutoCompile = false; var mt = model.Add(typeof(IntPtr), true); //mt.IncludeSerializerMethod = true; mt.SerializerType = typeof(Int64Serializer);

I do NOT see "No serializer defined for type: System.IntPtr" error anymore.

But now I see a new error which is =>

System.NotSupportedException: Specified method is not supported. at ProtoBuf.Grpc.Internal.Proxies.ClientBase.IEvAlertManagedCppSharpService_Proxy_0.IEvAlertManagedCppSharpService.setCommCellId(setCommCellIdRequest , CallContext )

gRPC client code var channel = GrpcChannel.ForAddress("https://localhost:6060", new GrpcChannelOptions { Credentials = new SslCredentials(), HttpClient = httpClient }); var client2 = channel.CreateGrpcService\<IEvAlertManagedCppSharpService>(); var ret2 = client2.setCommCellId(new setCommCellIdRequest { commCellId = 1, m_pEvAlertObj = IntPtr.Zero, isDisposed =false }).GetAwaiter().GetResult();

Interface code [ServiceContract]
public interface IEvAlertManagedCppSharpService { [OperationContract] public ValueTask\<setCommCellIdResponse> setCommCellId (setCommCellIdRequest request, CallContext context = default); }

contract public class setCommCellIdRequest {

[ProtoMember(1)]
public  Int64  commCellId { get; set; }

[ProtoMember(2)]
public  IntPtr  m_pEvAlertObj { get; set; }

[ProtoMember(3)]
public  Boolean  isDisposed { get; set; }

}

@mgravell can you please check this. ( BTW, I'm using protobuf-net V3)

mgravell commented 1 year ago

I'm looking at protobuf-net in isolation first, and it is working fine with the following:

public sealed class IntPtrSerializer : ISerializer<nint>, ISerializer<nuint>
        {
            public static void Configure(RuntimeTypeModel model)
            {
                model.Add<nint>(false).SerializerType = typeof(IntPtrSerializer);
                model.Add<nuint>(false).SerializerType = typeof(IntPtrSerializer);
            }

            public SerializerFeatures Features
                => SerializerFeatures.CategoryScalar | SerializerFeatures.WireTypeVarint;
            public nint Read(ref ProtoReader.State state, IntPtr value)
                => new IntPtr(state.ReadInt64());
            public UIntPtr Read(ref ProtoReader.State state, UIntPtr value)
                => new UIntPtr(state.ReadUInt64());
            public void Write(ref ProtoWriter.State state, IntPtr value)
                => state.WriteInt64(value.ToInt64());
            public void Write(ref ProtoWriter.State state, UIntPtr value)
                => state.WriteUInt64(value.ToUInt64());
        }

Note: I don't recommend changing AutoCompile - that's mostly useful for debugging purposes. Likewise, unless you're doing something exotic, you probably don't want custom model instances - because if you do that, you need to configure the gRPC bindings with that custom model. Again, my suggestion would be to configure the default model - which with the above would be, during startup (before any serialization code):

IntPtrSerializer.Configure(RuntimeTypeModel.Default);

I'll try to have a quick look at an end-to-end gRPC example / test

mgravell commented 1 year ago

end-to-end sample added; I gather (from now-deleted comment?) that you're sorted, so: great

I'll try to add native support in a protobuf-net tweak; when you next update protobuf-net, check the release notes: if it mentions nint/nuint/IntPtr/UIntPtr support: you'll probably have to remove your tweak: but I'll make sure the format is compatible

shivkumarghatbale commented 1 year ago

will check & update tomorrow. (Working from India).

Btw, @mgravell , how do I get rid-off following warnings which gets displayed at the time of start of grpc service. As you already know, the request setCommCellIdRequest has IntPtr, probably due to that only we've these warnings

warn: ProtoBuf.Grpc.Server.ServicesExtensions.CodeFirstServiceMethodProvider[0] Type cannot be serialized; ignoring: CVGrpcCppSharpLib.EvAlertManagedCppSharpContract.setCommCellIdRequest warn: ProtoBuf.Grpc.Server.ServicesExtensions.CodeFirstServiceMethodProvider[0] Signature not recognized for CVGrpcCppSharpLib.EvAlertManagedCppSharpContract.IEvAlertManagedCppSharpService.setCommCellId; method will not be bound

mgravell commented 1 year ago

are you sure that setCommCellIdRequest is marked as [ProtoContract] ?

mgravell commented 1 year ago

to be clear: those messages are important - that means the server isn't going to respond to that method; have you configured RuntimeTypeModel early enough?

shivkumarghatbale commented 1 year ago

checking

shivkumarghatbale commented 1 year ago

@mgravell You're right. I had configured RuntimeTypeModel very late. Moved it at the beginning.

  1. I do not see those warning messages at the start of grpc server :-)
  2. The entire client & server grpc call is working for IntPtr with the use of IntPtrSerializer you had suggested :-)

Thanks a lot!!! It worked all the way !!!! Please enjoy one more coffee which is on the way :-)

mgravell commented 1 year ago

nice; glad you got it working, and: you didn't need to change your models!

as a tip: you may (or may not, YMMV) find it helpful to tweak those properties to nint, i.e.

[ProtoMember(2)]
public nint m_pEvAlertObj { get; set; }

This is functionally identical, and is still IntPtr - nothing has changed, except the compiler now treats it more like a number than as a pointer. But it hasn't changed; this is just compiler magic.