https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6937
Mark Martinec <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|Undefined |3.4.0 --- Comment #3 from Mark Martinec <[email protected]> --- > I'm running trunk (3.4) under 5.18.0 and am not seeing this, > although I can't pinpoint any change which had been addressing > this problem. Will investigate... Took me two days to grasp it all. It is true that the current trunk did not suffer from this problem, the reason is my changes to DNS resolving (grouping of DNS queries & caching DNS responses for future use - r1434360, Bug 6884) - which unintentionally also avoided inserting entries into a hash while an each() context is active. What happens is that SA 3.3 invoked callbacks of finished DNS queries from MS::AsyncLoop::complete_lookups within an for/each loop. Some of the callbacks (e.g. NS lookup) could generate another DNS query (for an A record), which in turn inserted a new entry into a hash of pending queries. If this happened within an active each() context, older perl would just quietly suffer and possibly do something unpredictable (or maybe not), while the perl 5.18.0 issues a warning. The attached patch for 3.3.2 is just a quick workaround, building a list of keys of a 'pending' hash before the loop is entered. Changes by (r1434360, Bug 6884) moved calling of callback subroutines from MS::AsyncLoop::complete_lookups into MS::DnsResolver::poll_responses, i.e. somewhat earlier, but also outside of the each() context, so it avoided the problem altogether. The main reason for this change was re-using results of DNS queries when multiple independent rules happen to query the same DNS resources. A potential downside of the current approach of invoking callbacks earlier is that actions are more intertwined in comparison to decoupled calls to callback, delayed till after the poll_responses loop has already exited. ... which made me try to revert to the decoupled approach, but it all ended up in a mess of solving unnecessary new problems, so I finally gave up and kept the new approach. As a result of this exercise I updated some documentation, added/adjusted some debugging and added some hardening, and re-introduced the 'aborted' callbacks, even though they are not currently used by any callback. Also ditched some of the dead code: currently unused poll_callback and get_pending_lookups(), and the 'completed_callback' mechanism of specifying callbacks which has been superseded by the $cb argument of sub bgsend_and_start_lookup. trunk (3.4): Bug 6937: 3.3.2 and Perl 5.18.0: Altering hash requires restarting loop else UNDEFINED behavior Sending lib/Mail/SpamAssassin/AsyncLoop.pm Sending lib/Mail/SpamAssassin/Dns.pm Sending lib/Mail/SpamAssassin/DnsResolver.pm Sending lib/Mail/SpamAssassin/Plugin/ASN.pm Sending lib/Mail/SpamAssassin/Plugin/AskDNS.pm Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm Committed revision 1487178. -- You are receiving this mail because: You are the assignee for the bug.
