Skip to content

Commit

Permalink
[misc] solving possible race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
rusher committed Apr 10, 2017
1 parent b139f06 commit 128675a
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 103 deletions.
11 changes: 7 additions & 4 deletions src/main/java/org/mariadb/jdbc/MariaDbConnection.java
Expand Up @@ -50,8 +50,9 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS

package org.mariadb.jdbc;

import org.mariadb.jdbc.internal.logging.Logger;
import org.mariadb.jdbc.internal.logging.LoggerFactory;
import org.mariadb.jdbc.internal.protocol.Protocol;
import org.mariadb.jdbc.internal.com.read.dao.Results;
import org.mariadb.jdbc.internal.util.*;
import org.mariadb.jdbc.internal.util.dao.CallableStatementCacheKey;
import org.mariadb.jdbc.internal.util.dao.CloneableCallableStatement;
Expand All @@ -69,6 +70,7 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS


public class MariaDbConnection implements Connection {
private static Logger logger = LoggerFactory.getLogger(MariaDbConnection.class);

/**
* Pattern to check the correctness of callable statement query string
Expand Down Expand Up @@ -100,6 +102,7 @@ public class MariaDbConnection implements Connection {
private boolean canUseServerTimeout = false;
private boolean sessionStateAware = true;


/**
* save point count - to generate good names for the savepoints.
*/
Expand Down Expand Up @@ -728,6 +731,7 @@ public boolean isReadOnly() throws SQLException {
*/
public void setReadOnly(final boolean readOnly) throws SQLException {
try {
logger.debug("setReadOnly to value " + readOnly);
protocol.setReadonly(readOnly);
} catch (SQLException e) {
ExceptionMapper.throwException(e, this, null);
Expand Down Expand Up @@ -1100,9 +1104,8 @@ public boolean isValid(final int timeout) throws SQLException {
if (timeout < 0) {
throw new SQLException("the value supplied for timeout is negative");
}
if (isClosed()) {
return false;
}
if (isClosed()) return false;

try {
return protocol.ping();
} catch (SQLException e) {
Expand Down

This file was deleted.

Expand Up @@ -130,6 +130,7 @@ public HandleErrorResult primaryFail(Method method, Object[] args) throws Throwa
boolean alreadyClosed = !currentProtocol.isConnected();
boolean inTransaction = currentProtocol != null && currentProtocol.inTransaction();

proxy.lock.lock();
try {
if (currentProtocol != null && currentProtocol.isConnected() && currentProtocol.ping()) {
//connection re-established
Expand All @@ -141,15 +142,12 @@ public HandleErrorResult primaryFail(Method method, Object[] args) throws Throwa
return new HandleErrorResult(true);
}
} catch (SQLException e) {
proxy.lock.lock();
try {
currentProtocol.close();
} finally {
proxy.lock.unlock();
}
currentProtocol.close();
if (setMasterHostFail()) {
addToBlacklist(currentProtocol.getHostAddress());
}
} finally {
proxy.lock.unlock();
}

try {
Expand Down
Expand Up @@ -441,12 +441,9 @@ public void switchReadOnlyConnection(Boolean mustBeReadOnly) throws SQLException
if (currentReadOnlyAsked != mustBeReadOnly) {
proxy.lock.lock();
try {
if (currentReadOnlyAsked == mustBeReadOnly) {
// someone else updated state
return;
} else {
currentReadOnlyAsked = mustBeReadOnly;
}
// another thread updated state
if (currentReadOnlyAsked == mustBeReadOnly) return;
currentReadOnlyAsked = mustBeReadOnly;
if (currentReadOnlyAsked) {
if (currentProtocol.isMasterConnection()) {
//must change to replica connection
Expand Down Expand Up @@ -529,7 +526,9 @@ public void switchReadOnlyConnection(Boolean mustBeReadOnly) throws SQLException
public HandleErrorResult primaryFail(Method method, Object[] args) throws Throwable {
boolean alreadyClosed = !masterProtocol.isConnected();
boolean inTransaction = masterProtocol != null && masterProtocol.inTransaction();

//try to reconnect automatically only time before looping
proxy.lock.lock();
try {
if (masterProtocol != null && masterProtocol.isConnected() && masterProtocol.ping()) {
if (inTransaction) {
Expand All @@ -539,16 +538,13 @@ public HandleErrorResult primaryFail(Method method, Object[] args) throws Throwa
return relaunchOperation(method, args);
}
} catch (SQLException e) {
proxy.lock.lock();
try {
masterProtocol.close();
} finally {
proxy.lock.unlock();
}
masterProtocol.close();

if (setMasterHostFail()) {
addToBlacklist(masterProtocol.getHostAddress());
}
} finally {
proxy.lock.unlock();
}

//fail on slave if parameter permit so
Expand Down Expand Up @@ -634,18 +630,20 @@ public void reconnect() throws SQLException {
}
}

/**
* Ping secondary protocol.
* ! lock must be set !
*
* @param protocol socket to ping
* @return true if ping is valid.
*/
private boolean pingSecondaryProtocol(Protocol protocol) {
try {
if (protocol != null && protocol.isConnected() && protocol.ping()) {
return true;
}
} catch (Exception e) {
proxy.lock.lock();
try {
protocol.close();
} finally {
proxy.lock.unlock();
}
protocol.close();

if (setSecondaryHostFail()) {
addToBlacklist(protocol.getHostAddress());
Expand All @@ -663,10 +661,16 @@ private boolean pingSecondaryProtocol(Protocol protocol) {
* @throws Throwable if failover has not catch error
*/
public HandleErrorResult secondaryFail(Method method, Object[] args) throws Throwable {
if (pingSecondaryProtocol(this.secondaryProtocol)) {
return relaunchOperation(method, args);
proxy.lock.lock();
try {
if (pingSecondaryProtocol(this.secondaryProtocol)) {
return relaunchOperation(method, args);
}
} finally {
proxy.lock.unlock();
}


if (!isMasterHostFail()) {
try {
if (masterProtocol != null) {
Expand Down
Expand Up @@ -780,8 +780,10 @@ public boolean ping() throws SQLException {
lock.lock();
try {

final SendPingPacket pingPacket = new SendPingPacket();
pingPacket.send(writer);
writer.startPacket(0);
writer.write(COM_PING);
writer.flush();

Buffer buffer = reader.getPacket(true);
return buffer.getByteAt(0) == OK;

Expand Down

0 comments on commit 128675a

Please sign in to comment.