[bug #58550] WebServer - Deadlock due to missing socketAddress

[David Ayers]

URL:
<https://savannah.gnu.org/bugs/?58550>

Summary: WebServer - Deadlock due to missing socketAddress
Project: GNUstep
Submitted by: ayers
Submitted on: Fri 12 Jun 2020 11:29:31 AM CEST
Category: Libraries
Severity: 3 - Normal
Item Group: Bug
Status: None
Privacy: Public
Assigned to: None
Open/Closed: Open
Discussion Lock: Any

_______________________________________________________

Details:

The WebServer method _didConnect retrieves the foreign socketAddress from the
file handle. The refusal message addresses the possibility of that address
being nil. Yet later the variable for the address is unconditionally added
the the _perHost set which raises an Exception while holding the WebServer
lock, leading to a deadlock upon the next request.

Problem posting notification: <NSException: 0x55e7b5b0fd80>
NAME:NSInvalidArgumentException REASON:Tried to nil value to counted set
INFO:(null)
*** -[NSLock lock]: deadlock (<NSLock: 0x55e7a9a75ed0> (locked by 19025))
*** Break on _NSLockError() to debug.

Later in _endConnect the address is also conditionally removed again, but that
only invokes a warning if address is nil.

I do not know what the intended behavior is but I've fixed/worked around the
problem, by simply checking for nil and not adding or removing the nil value
to or from the set.




_______________________________________________________

Reply to this item at:

<https://savannah.gnu.org/bugs/?58550>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

[Richard Frith-Macdonald]

Update of bug #58550 (project gnustep):

Status: None => Confirmed

_______________________________________________________

Follow-up Comment #1:

Thanks ... clearly a bug ... I'll fix that now.

[Richard Frith-Macdonald]

Follow-up Comment #2, bug #58550 (project gnustep):

Actually, it looks like it was already fixed long ago: time to make a release
perhaps.

However, the current code, while adding @"unknown" to the counted set (and
thus avoiding the exception and locking issue), is removing nil at the end of
the connection, which of course means that the counted set will get the number
of connections for an 'unknown' host wrong. That's a much less important bug,
but still a bug.

I will change the code to use 'unknown' as the host name consistently.

[David Ayers]

Update of bug #58550 (project gnustep):

Status: Confirmed => None

_______________________________________________________

Follow-up Comment #3:

Indeed, this was based off of a release tag. Thanks for the fix! And a new
release would be great.