1fanwang opened a new pull request, #3750: URL: https://github.com/apache/thrift/pull/3750
A c_glib service that extends nothing answers every method call with an application exception reading `Invalid method name`, so a base service is unusable while a derived one works. The generated class declares its own `dispatch_call` member, which children read as `parent_class->dispatch_call` when chaining up. That is a different field from the one on `ThriftDispatchProcessorClass`, and the latter is what the runtime actually calls. The generator registered that slot only for a service extending another one, leaving a base service on the inherited default, whose job is to reject method names nothing in the hierarchy recognises. The guard existed to skip a dispatch hop to a parent that does not exist. That hop is emitted in the generated dispatch body, which the guard never touched, so it saved no work and only cost base services their dispatch. Registering the slot unconditionally restores it. A service that extends another generates byte-identical output. ### Testing The regression case drives the generated processor for `Srv`, which extends nothing, and asserts both that the handler was reached and that the reply carries its return value. ``` cd build && ctest -R testdispatchprocessor --output-on-failure ``` With only the generator reverted, regenerating and rebuilding first: ``` git checkout HEAD~1 -- compiler/cpp/src/thrift/generate/t_c_glib_generator.cc rm -rf build/lib/c_glib/test/gen-c_glib && make -j8 thrift-compiler testdispatchprocessor ``` ``` ok 4 /testdispatchprocessor/DispatchCallWithoutName ERROR:lib/c_glib/test/testdispatchprocessor.c:451:test_generated_base_service_dispatches: assertion failed: (srv_handler_reached == TRUE) not ok /testdispatchprocessor/GeneratedBaseServiceDispatches Bail out! 0% tests passed, 1 tests failed out of 1 ``` Restoring the generator and regenerating: ``` git checkout HEAD -- compiler/cpp/src/thrift/generate/t_c_glib_generator.cc rm -rf build/lib/c_glib/test/gen-c_glib && make -j8 thrift-compiler testdispatchprocessor cd build && ctest -R testdispatchprocessor ``` ``` 1/1 Test #29: testdispatchprocessor ............ Passed 0.69 sec 100% tests passed out of 1 ``` Verified under cmake on macOS arm64 only. The `Makefile.am` change follows the pattern `testdebugproto_LDADD` already uses and the library already compiles the needed source, so I expect autotools to be fine, but that is reasoning from the existing pattern rather than something I ran. Worth a CI run before merge. `ThriftCompilerTests` does not build on this platform, because of inline assembly in the cpp generator tests, and that is unrelated to this change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
