On 07/14/2014 03:48 AM, Amos Jeffries wrote:
>   Converts the PortCfgPointer to reference counted

This change should have gone through an audit IMO, but I appreciate
having an entire Sunday to react to the "will commit shortly" notice for
a not yet posted patch.


> -    if (!s.valid()) {
> +    if (!s) {
>          // it is possible the call or accept() was still queued when the 
> port was reconfigured
>          debugs(33, 2, "HTTP accept failure: port reconfigured.");
>          return;

While the port pointer could get invalidated in the [fixed] old code, it
cannot become nil in the new code. The above comment is no longer valid
and the code itself no longer works as intended. Please fix both
instances of the above logic: one in httpAccept() and one in httpsAccept().

AFAICT, if you simply replace the above code with a comment warning that
the port may no longer be in the current configuration, we will continue
to start transactions previously accepted on no longer configured ports.
Hopefully, this will not cause any new problems, but there may be a
better solution.


Thank you,

Alex.

Reply via email to