Skip to content

Close socket with :abort T after error#150

Open
vibs29 wants to merge 1 commit intoedicl:masterfrom
vibs29:close_abort_after_error
Open

Close socket with :abort T after error#150
vibs29 wants to merge 1 commit intoedicl:masterfrom
vibs29:close_abort_after_error

Conversation

@vibs29
Copy link

@vibs29 vibs29 commented Jan 10, 2026

clisp (or less likely usocket) doesn't allow socket operations, other than close, after an error; it exits with code 141. CL+SSL might produce an error e.g. during implicit force-outputs. DRAKMA's http-request then calls its close method in an unwind-protect cleanup form, which calls force-output again. This second call causes clisp to exit. The following semantics for close seem sensible: If you've received an error operating with a plain-text or CL+SSL socket, then call close with :abort T. Use :abort NIL only when you've encountered no errors. When close is used that way, the problem gets solved. CL+SSL's close doesn't call force-output when :abort is T.

clisp (or less likely usocket) doesn't allow socket operations, other
than close, after an error; it exits with code 141.  CL+SSL might produce
an error e.g. during implicit force-outputs. DRAKMA's http-request then
calls its close method in an unwind-protect cleanup form, which calls
force-output again. This second call causes clisp to exit.  The following
semantics for close seem sensible: If you've received an error operating
with a plain-text or CL+SSL socket, then call close with :abort T.
Use :abort NIL only when you've encountered no errors. When close is
used that way, the problem gets solved. CL+SSL's close doesn't call
force-output when :abort is T.
@hanshuebner
Copy link
Member

I would like to see this fixed, but the logic was already difficult to understand, and now with the additional flet and cond it really exceeds my brain capacity. Can you find a way to make this easier to review?

@stassats
Copy link
Member

Or maybe clisp should be fixed?

@vibs29
Copy link
Author

vibs29 commented Jan 10, 2026

I don't fully understand the original cleanup clause, but here is what I did. The changed version is equivalent to the original except that in one case it closes with abort T instead of abort NIL.

This was the original cleanup clause.

(when (and http-stream
           (or (not done)
               (and must-close
                    (not want-stream)))
           (not (eq content :continuation)))
  (ignore-errors (close http-stream)))

which, swapping the last two outer AND terms, is equivalent to

(when (and http-stream
           (not (eq content :continuation))
           (or (not done)
               (and must-close
                    (not want-stream))))
  (ignore-errors (close http-stream)))

which is

(when (and http-stream
           (not (eq content :continuation)))
  (when (or (not done)
            (and must-close
                 (not want-stream)))
    (ignore-errors (close http-stream))))

which is

(when (and http-stream
           (not (eq content :continuation)))
  (cond ((not done) (ignore-errors ...))
        ((and must-close (not want-stream)) (ignore-errors ...))))

which is

(when (and http-stream
           (not (eq content :continuation)))
  (flet ((close-aborting (a) (ignore-errors (close http-stream :abort a))))
    (cond ((not done) (close-aborting nil))
          ((and must-close (not want-stream)) (close-aborting nil)))))

I then finally changed the behaviour to what I wanted, which was to change the NIL to T in the (not done) situation. (That situation represents that an error occurred in the unwind-protect's body.)

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

Successfully merging this pull request may close these issues.

3 participants