Chromium Code Reviews (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out

Issue 2540013002: VM: Fix segfault during shutdown of socket listening registry (Closed)

4 years ago by kustermann
4 years ago
Target Ref:


VM: Fix segfault during shutdown of socket listening registry The following happened: A listening socket (ipv4, first-port) was created. Afterwards another listening socket (ipv6, 0) is being created. We look up in the hashmap if there is already a listening socket on port `0` (which is never the case). So we create a new socket, the OS assigns the socket a random port (which just happens to be `first-port` as well). Then we insert a new OSSocket entry into the hashmap with key `first-port` (and set its `next` field to NULL). This will simply override the existing entry. During shutdown we loop over all (fd, OSSocket) pairs and try to remove `OSSocket` from the port->socket map (via the linked `next` list). We cannot find the `OSSocket` which we accidentally droped earlier, so we dereference `0->next` and hit a SEGFAULT. What should've happend is to correctly link the `OSSocket`s via the next field. Fixes #27847, Committed:

Patch Set 1 #

Total comments: 4

Patch Set 2 : addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -35 lines) Patch
M runtime/bin/ View 1 2 chunks +57 lines, -35 lines 0 comments Download


Total messages: 11 (5 generated)
4 years ago (2016-11-29 23:28:40 UTC) #2
DBQ: possible to create a regression test for this? Seems like something we should avoid ...
4 years ago (2016-11-29 23:58:37 UTC) #4
Florian Schneider
Lgtm. Maybe add "fixes #27847" to the CL description. @kevmoo: It may not be possible ...
4 years ago (2016-11-30 00:21:02 UTC) #6
lgtm. Agree that a reliable regression test here is a problem as the test has ...
4 years ago (2016-11-30 00:41:29 UTC) #7
Committed patchset #2 (id:20001) manually as f9c9c4e767e4b9538cf6e76e7327b7683235a06e (presubmit successful).
4 years ago (2016-11-30 10:48:15 UTC) #10
4 years ago (2016-11-30 10:51:11 UTC) #11
Message was sent while issue was closed.
@kevmoo: As florian/siva already mentioned, this is an issue which only occurs
precisely if we allow the kernel to choose a new random port for a listening
socket and that very same port is already being listened upon by dart code (on a
different address).
File runtime/bin/ (right):
runtime/bin/ MutexLocker ml(ListeningSocketRegistry::mutex_);
On 2016/11/30 00:21:01, Florian Schneider wrote:
> nit: no need to use qualified name here
> MutexLocker ml(mutex_);
> should be enough?

runtime/bin/ ASSERT(port == 0);
On 2016/11/30 00:21:01, Florian Schneider wrote:
> Maybe add a short comment here about the scenario you described in the CL
> description.


Powered by Google App Engine
This is Rietveld 408576698