Bug 116873 - [Qt][WK2] Make the work queue take the ownership of the socket descriptor
Summary: [Qt][WK2] Make the work queue take the ownership of the socket descriptor
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-28 09:59 PDT by Carlos Garcia Campos
Modified: 2022-02-28 03:57 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.64 KB, patch)
2013-06-11 16:46 PDT, Lamarque V. Souza
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-05-28 09:59:29 PDT
We have found several race conditions with the connection termination in the GTK+ port and have switched to make the work queue the only one responsible for closing the socket. We added another platform ifdef to platformInvalidate in ConnectionUnix.cpp. See bug #115880 for more details. 

Both Anders and Martin suggested that to avoid race conditions this should also be the case of other ports, and the connection queue should take the ownership of the socket descriptor. That way we can get rid of the platform ifdef added to ConnectionUnix.
Comment 1 Lamarque V. Souza 2013-06-11 16:46:36 PDT
Created attachment 204370 [details]
Patch

Proposed patch. By what I can see only Connection::platformInitialize() closes the socket descriptor in the Qt port. This patch moves the loop closing the socket descriptor to SocketNotifier's destructor. SocketNotifier is a new class that inherits from QSocketNotifier. SocketNotifier is destroyed a few lines below the loop in Connection::platformInvalidate(), so this change should keep things working the same as before. I tested this change with the 'Test case' patch in https://bugs.webkit.org/show_bug.cgi?id=115880 and it passes.
Comment 2 Anders Carlsson 2013-10-02 21:33:33 PDT
Comment on attachment 204370 [details]
Patch

Qt has been removed, clearing review flags.
Comment 3 Jocelyn Turcotte 2014-02-03 03:25:51 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.