Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/src/server/handlers.dart 's _customHeaders is reset to null, it should be reset to empty Map instead #736

Open
isoos opened this issue Sep 19, 2024 · 7 comments

Comments

@isoos
Copy link

isoos commented Sep 19, 2024

Error report:
https://www.reddit.com/r/dartlang/comments/1fk84bb/unhandled_exception_from_grpc_package/

The reported line seems to be when forcing _customHeaders to not-null: https://github.com/grpc/grpc-dart/blob/master/lib/src/server/handler.dart#L384

Looking at the git blame history, I think _customHeaders = null should have been migrated to _customHeaders = <String, String>{} when the null-safety migration happened.

@mraleph
Copy link
Member

mraleph commented Sep 19, 2024

I think the current code is correct. You can only send headers once so they are intentionally reset to null after that. You should not be able to enter this code again (it is additionally guarded by checking _headersSent).

I also see that the benchmark code in question is referencing Dart 2.19 and pubspec.lock mentions grpc version 3.1. This is really old stuff.

We will need to know exact stack trace and exact version of grpc package for this to be actionable.

For now I am going to close this - but if r/David_Owens provides necessary info I can reopen.

@mraleph mraleph closed this as completed Sep 19, 2024
@owensdj
Copy link

owensdj commented Sep 19, 2024

Here is a screen shot of the stack trace. It's running in a Docker container and only comes up for a split second, so I had to take a video of it.

https://1drv.ms/i/c/29ea63e9e7ad0202/EQ3bEw8Cer1KrEQHk-vpYmABnqFhUoNf85uwvdy7C0MLqQ?e=oGMD3A

0 ServerHandler.sendTrailers (package:grpc/src/server/handler.dart:384)
1 ServerHandler._sendError (package:grpc/src/server/handler.dart:459)
2 ServerHandler._onResponse (package:grpc/src/server/handler.dart:327)
...

@mraleph
Copy link
Member

mraleph commented Sep 19, 2024

@owensdj what is the version of the package / Dart SDK you are using?

@owensdj
Copy link

owensdj commented Sep 19, 2024

gRPC 4.0.1 and Dart SDK 3.5.2. The problem happened with older versions as well.

@mraleph
Copy link
Member

mraleph commented Sep 19, 2024

One possible issue is that here https://github.com/grpc/grpc-dart/blob/master/lib/src/server/handler.dart#L360-L367 _stream.sendHeaders can trigger an error at which point _customHeaders will end up null but _headersSent is not true. This might be an underlying root cause.

@mraleph mraleph reopened this Sep 19, 2024
@owensdj
Copy link

owensdj commented Sep 19, 2024

@mraleph That makes sense, but if you look at the call stack I don't see where the execution would have set _customHeaders to null. The only places where it's ever set to null is in sendHeaders on line 361 and in sendTrailers on line 388. I don't see sendHeaders in the call stack and it crashes at line 384 in sendTrailers before it gets to the line setting _customHeaders to null.

@mraleph
Copy link
Member

mraleph commented Sep 20, 2024

That is correct that we don't see sendHeaders in the stack trace, but we see _sendError and a bunch of async machinery. It indicates that some sort of error happened and we are trying to propagate it. Also the explanation which I gave is based on the logic - that's the only possible sequence of events which leads to this.

If I put throw 'aaa'; right before sendHeaders then this causes exactly the type of a crash that you have reported:

Unhandled exception:
Null check operator used on a null value
#0      ServerHandler.sendTrailers (package:grpc/src/server/handler.dart:385:21)
#1      ServerHandler._sendError (package:grpc/src/server/handler.dart:460:5)
#2      ServerHandler._onResponse (package:grpc/src/server/handler.dart:327:7)
#3      _RootZone.runUnaryGuarded (dart:async/zone.dart:1609:10)
#4      _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:366:11)
#5      _BufferingStreamSubscription._add (dart:async/stream_impl.dart:297:7)
#6      _SyncStreamControllerDispatch._sendData (dart:async/stream_controller.dart:777:19)
#7      _StreamController._add (dart:async/stream_controller.dart:651:7)
#8      new Stream.fromFuture.<anonymous closure> (dart:async/stream.dart:249:18)
#9      Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:902:45)
#10     Future._propagateToListeners (dart:async/future_impl.dart:931:13)
#11     Future._chainCoreFutureSync (dart:async/future_impl.dart:632:7)
#12     Future._chainCoreFutureAsync.<anonymous closure> (dart:async/future_impl.dart:677:7)
#13     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
#14     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)
#15     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)
#16     _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:185:5)

So I think this is the most possible explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants