Skip to content

Commit 7899a60

Browse files
committed
Merge pull request brianmario#463 from eam/master
Don’t shutdown() shared sockets (fixes brianmario#213)
2 parents a74e119 + a2bcfd6 commit 7899a60

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)