After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 721074 - kqueue: deadlock
kqueue: deadlock
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.38.x
Other OpenBSD
: Normal major
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-12-26 09:14 UTC by Antoine Jacoutot
Modified: 2013-12-26 21:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
only lock when we add a monitor (1.08 KB, patch)
2013-12-26 09:14 UTC, Antoine Jacoutot
none Details | Review
unlock before returning (927 bytes, patch)
2013-12-26 10:05 UTC, Antoine Jacoutot
committed Details | Review

Description Antoine Jacoutot 2013-12-26 09:14:40 UTC
Created attachment 264894 [details] [review]
only lock when we add a monitor

Hi.

There is a potential deadlock in the kqueue monitoring code which I was able to reproduce by just running gedit.
In case an object is already monitored, we lock then return without unlocking it which can then result in a self deadlock.
Moving the check _before_ locking works for me.
Note that this only happens when your default mutex type is PTHREAD_MUTEX_STRICT_NP (which is the case on BSDx).

Toughts?
Comment 1 Dmitry Matveev 2013-12-26 09:30:24 UTC
Hi Antoine,

Yes, definetely it is a bug.

But the proper solution is to place a new G_UNLOCK before that return, instead of moving G_LOCK.

The `missing_subs_list` variable may be concurrently accessed from multiple threads thus it has to be protected with a lock.

Dmitry
Comment 2 Antoine Jacoutot 2013-12-26 10:05:04 UTC
> Yes, definetely it is a bug.
> 
> But the proper solution is to place a new G_UNLOCK before that return, instead
> of moving G_LOCK.
> 
> The `missing_subs_list` variable may be concurrently accessed from multiple
> threads thus it has to be protected with a lock.

Sure.
Here's a new diff, hopefully this is what you had in mind.
Thanks.
Comment 3 Antoine Jacoutot 2013-12-26 10:05:33 UTC
Created attachment 264895 [details] [review]
unlock before returning
Comment 4 Allison Karlitskaya (desrt) 2013-12-26 18:44:31 UTC
Review of attachment 264895 [details] [review]:

This is obviously correct and if it fixes the issue, then please commit it.

Thanks!
Comment 5 Antoine Jacoutot 2013-12-26 18:59:34 UTC
(In reply to comment #4)
> Review of attachment 264895 [details] [review]:
> 
> This is obviously correct and if it fixes the issue, then please commit it.
> 
> Thanks!

Thank you Ryan. It is in: fb21c8eaab64301591a0e179a42ef25cbd793a6e
I think it'd be worth to add it to your list of cherry picks for glib-2-38.
It's in OpenBSD ports already.
Comment 6 Allison Karlitskaya (desrt) 2013-12-26 21:05:24 UTC
Comment on attachment 264895 [details] [review]
unlock before returning

Picked it over just now, thanks.