Skip to content

Commit a2bcfd6

Browse files
committed
Avoid shutdown() on shared sockets (fixes brianmario#213)
Close the mysql socket before calling mysql_close(). mysql_close() calls shutdown() on the attached socket. In a situation involving fork() and copies of an object spanning multiple interpreter instances this will result in the first instance to GC breaking all other instances sharing the socket. dylanahsmith notes that we can avoid this by simply closing the socket before entering mysql_close(), preventing the shutdown() from having any impact on the shared socket state.
1 parent f19ff01 commit a2bcfd6

File tree

2 files changed

+29
-15
lines changed

2 files changed

+29
-15
lines changed

ext/mysql2/client.c

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,26 +164,19 @@ static void *nogvl_connect(void *ptr) {
164164

165165
static void *nogvl_close(void *ptr) {
166166
mysql_client_wrapper *wrapper;
167-
#ifndef _WIN32
168-
int flags;
169-
#endif
170167
wrapper = ptr;
171168
if (wrapper->connected) {
172169
wrapper->active_thread = Qnil;
173170
wrapper->connected = 0;
174-
/*
175-
* we'll send a QUIT message to the server, but that message is more of a
176-
* formality than a hard requirement since the socket is getting shutdown
177-
* anyways, so ensure the socket write does not block our interpreter
178-
*
179-
*
180-
* if the socket is dead we have no chance of blocking,
181-
* so ignore any potential fcntl errors since they don't matter
182-
*/
183171
#ifndef _WIN32
184-
flags = fcntl(wrapper->client->net.fd, F_GETFL);
185-
if (flags > 0 && !(flags & O_NONBLOCK))
186-
fcntl(wrapper->client->net.fd, F_SETFL, flags | O_NONBLOCK);
172+
/* Call close() on the socket before calling mysql_close(). This prevents
173+
* mysql_close() from sending a mysql-QUIT or from calling shutdown() on
174+
* the socket. The difference is that close() will drop this process's
175+
* reference to the socket only, while a QUIT or shutdown() would render
176+
* the underlying connection unusable, interrupting other processes which
177+
* share this object across a fork().
178+
*/
179+
close(wrapper->client->net.fd);
187180
#endif
188181

189182
mysql_close(wrapper->client);

spec/mysql2/client_spec.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,27 @@ def connect *args
126126
final_count.should == before_count
127127
end
128128

129+
if Process.respond_to?(:fork)
130+
it "should not close connections when running in a child process" do
131+
GC.start
132+
sleep 1 if defined? Rubinius # Let the rbx GC thread do its work
133+
client = Mysql2::Client.new(DatabaseCredentials['root'])
134+
135+
fork do
136+
client.query('SELECT 1')
137+
client = nil
138+
GC.start
139+
sleep 1 if defined? Rubinius # Let the rbx GC thread do its work
140+
end
141+
142+
Process.wait
143+
144+
# this will throw an error if the underlying socket was shutdown by the
145+
# child's GC
146+
expect { client.query('SELECT 1') }.to_not raise_exception
147+
end
148+
end
149+
129150
it "should be able to connect to database with numeric-only name" do
130151
lambda {
131152
creds = DatabaseCredentials['numericuser']

0 commit comments

Comments
 (0)