K0K0V0K commented on PR #8665: URL: https://github.com/apache/hadoop/pull/8665#issuecomment-5250753346
Regarding the one spotbug warning: > Class org.apache.hadoop.mcp.McpHttpServlet defines non-transient non-serializable instance field requestHandler In McpHttpServlet.java:instance field requestHandler In McpHttpServlet.java This warning is expected given how servlets inherit Serializable. It is not a functional bug in the MCP implementation. **What triggers the warning** McpHttpServlet extends HttpServlet, which implements Serializable. Static analysis therefore expects every non-transient instance field to be serializable. requestHandler is a McpRequestHandler, which does not implement Serializable. It also holds tool handlers (lambdas/closures) that cannot be serialized. With transient removed from these fields, the analyzer correctly reports SE_BAD_FIELD / “non-transient non-serializable instance field”. This is the flip side of the earlier “transient field not restored on deserialization” warning: servlets are Serializable by inheritance, but MCP never serializes this servlet. **Why this is not a real issue** McpHttpServlet is only constructed in-process via McpServer.build() and registered with Jetty (ServletHolder). There is no code path that Java-serializes or deserializes this servlet. If deserialization ever happened, the servlet would be broken regardless — but that path does not exist in Hadoop’s MCP usage. The warning reflects the servlet API’s Serializable contract, not a runtime defect in this feature. **Could we “fix” it?** Yes, there is already a pattern for that in [hadoop](https://github.com/apache/hadoop/blob/dccedc252716e9d4b046e79cae7b5187104c25ac/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/WebAppProxyServlet.java#L727). mark collaborator fields transient, and add a readObject() that reinitializes them after defaultReadObject(). That would silence both serialization-related FindBugs rules. On the other hand i would rather avoid this solution due extra complexity. Of course if community want i can apply the above mentioned solution. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
