Closures in FieldInfo make it impossible to send some message representations to isolates

[timburks]

I got the following error message when I tried to send a protobuf message to an isolate:

Unhandled exception:
Invalid argument(s): Illegal argument in isolate message : (object is a closure - Function '<anonymous closure>':.)
#0      _SendPortImpl._sendInternal (dart:isolate/runtime/libisolate_patch.dart:218:70)
#1      _SendPortImpl.send (dart:isolate/runtime/libisolate_patch.dart:202:5)
#2      sendReceive (file:///Users/tim/Desktop/DART/messages/bin/printer.dart:40:8)
#3      main (file:///Users/tim/Desktop/DART/messages/bin/printer.dart:54:3)
<asynchronous suspension>
#4      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:287:32)
#5      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:171:12)

This surprised me because I had previously been able to send other message representations. Looking more closely, I found that the error only occurred for my messages with repeated fields, and looking into the representation of FieldInfo I think I see why: because this field and the ones following it store functions for various special cases, including repeated fields.

Here is my test program:

import 'dart:async';
import 'dart:isolate';

import 'package:printer/src/generated/sample.pb.dart';

// Runs in a separate thread.
void printer(SendPort listener) async {
  // create receive port and send it to the listener
  var receivePort = new ReceivePort();
  listener.send(receivePort.sendPort);
  
  // listen on receive port for things to print
  await for (var msg in receivePort) {
    print('>> [${msg[0]}] (${msg[0].runtimeType})');
	var replyPort = msg[1];
	replyPort.send(null);
  }
}

/// sends a message on a port, 
/// receives the response,
/// and returns a future for the message that was received
Future sendReceive(SendPort port, msg) {
  var response = new ReceivePort();
  port.send([msg, response.sendPort]);
  return response.first;
}

main(List<String> args) async {
  // Start the printer and wait for it to reply with its SendPort.
  var receivePort = new ReceivePort();
  Isolate.spawn(printer, receivePort.sendPort);
  var  p = await receivePort.first;

  // Send some things
  sendReceive(p, 123);
  sendReceive(p, "hello");
  sendReceive(p, Kiosk());
  sendReceive(p, SetSignIdForKioskIdsRequest());
}

My protos are in sample.proto:

syntax = "proto3";

package sample;

// Describes a hardware device that can display signs.
message Kiosk {
  int32 id = 1;                       // unique id
  string name = 2;                    // name of device hardware
  ScreenSize size = 3;                // screen size
}

// Represents the size of a screen in pixels.
message ScreenSize {
  int32 width = 1;                    // screen width
  int32 height = 2;                   // screen height
}

message SetSignIdForKioskIdsRequest {
  repeated int32 kiosk_ids = 1;
  int32 sign_id = 2;
}

When I remove the repeated keyword from the kiosks_ids field in my proto, my test program runs successfully:

dart bin/printer.dart 
>> [123] (int)
>> [hello] (String)
>> [] (Kiosk)
>> [] (SetSignIdForKioskIdsRequest)

I'm new to Dart, so I'm not sure that sending structs into isolates is a good practice (due to possible efficiency problems), but it seems bad for this to work for some protobuf messages and not others.

[mit-mit]

cc @szakarias

[sigurdm]

We are working on removing these closures from the messages.
#198 will get rid of the first

[sigurdm]

That was not entirely true. The closure is still there in the makeDefault.
We have just been discussing this with @mkustermann and it seems we can work around the closure by replacing it with a single-method object...

[cah4a]

It's stopping me from using protocol buffers with Flutter.

[mkustermann]

It's stopping me from using protocol buffers with Flutter.

If you're using isolates: A stop-gap for this would be to serialize to bytes before sending and deserialize on the receiving side.

[sigurdm]

If you're using isolates: A stop-gap for this would be to serialize to bytes before sending and deserialize on the receiving side.

That doesn't help though if you use isolates to avoid ui hick-ups when parsing :)

[cah4a]

If you're using isolates: A stop-gap for this would be to serialize to bytes before sending and deserialize on the receiving side.

That's a considerable disadvantage. If serialization or deserialization costs more than 16ms, there is no way to avoid UI-freezes.

[ToluwaniO]

Any updates on this? @sigurdm

[sigurdm]

No unfortunately.

[ryans233]

Came here for same reason... :(

[xing-zheng]

We are working on removing these closures from the messages.
#198 will get rid of the first

Hey @sigurdm Is there any updates on this? it's still not working with isolates or compute() function.

[mkustermann]

I'll try to take a stab at this with #450

[mit-mit]

@mkustermann, I think this can be closed?

[mkustermann]

@mit-mit This hasn't been published yet - not even as a dev release.

[mit-mit]

Ah, got it, thanks!

[peterweb2005]

any news?
thanks

[mkustermann]

The changes I made are included in the null_safety branch on the git repository (see https://github.com/dart-lang/protobuf/tree/null_safety)

It appears like package:protobuf version 2.0.0 as well as package:protoc_plugin version 20.0.0 were published earlier this year from this branch and should therefore contain all of those changes.

@peterweb2005 Have you tried using those versions?

[peterweb2005]

Sorry, will late home today,
so no verified.

I recall that this year flutter & dart had big update,
in order to fix the lint warning from Android studio
so I generate the protobuf dart data file again (use newest binary, windows),
and seems fixed the lint

Then few days ago, from now,, I just tried to migrate that parse process to compute,
but not work,
saw the error, and saw the data file had closure

[mkustermann]

We have a test checked into the protobuf repository that tests sending protos via SendPorts (see protoc_plugin/test/send_protos_via_sendports_test.dart)

@peterweb2005 If it doesn't work for you, could I ask you to make a small reproduction, so we can investigate?

[peterweb2005]

Used the latest libaries:
Package protoc_plugin is currently active at version 20.0.0
protoc-3.17.3-win64.zip

and generated dart data file has no change, ie: has "closure"

[peterweb2005]

We have a test checked into the protobuf repository that tests sending protos via SendPorts (see protoc_plugin/test/send_protos_via_sendports_test.dart)

@peterweb2005 If it doesn't work for you, could I ask you to make a small reproduction, so we can investigate?

D:\peter\w_dart\protobuf\protoc_plugin>dart test/send_protos_via_sendports_test.dart
00:00 +0: Normal proto can be transferred via ports

00:00 +1: Map-using proto can be transferred via ports

00:00 +2: All tests passed

[peterweb2005]

edit

i tested,
it is my bug

because i mistakenly thought the closure in protobuf data files cause the error

[osa1]

It appears like package:protobuf version 2.0.0 as well as package:protoc_plugin version 20.0.0 were published earlier this year from this branch and should therefore contain all of those changes.

This issue was fixed with #450 and the changes were released with protobuf-2.0.0 and protoc_plugin-20.0.0.