diff --git a/db/curop.h b/db/curop.h index c21478c1608..22b3871601d 100644 --- a/db/curop.h +++ b/db/curop.h @@ -150,6 +150,8 @@ namespace mongo { ThreadSafeString _message; ProgressMeter _progressMeter; + + volatile bool _killed; void _reset(){ _command = false; @@ -159,6 +161,7 @@ namespace mongo { _waitingForLock = false; _message = ""; _progressMeter.finished(); + _killed = false; } void setNS(const char *ns) { @@ -338,53 +341,57 @@ namespace mongo { string getMessage() const { return _message.toString(); } ProgressMeter& getProgressMeter() { return _progressMeter; } + void kill() { _killed = true; } + bool killed() const { return _killed; } + friend class Client; }; - /* 0 = ok - 1 = kill current operation and reset this to 0 - future: maybe use this as a "going away" thing on process termination with a higher flag value + /* _globalKill: we are shutting down + otherwise kill attribute set on specified CurOp + this class does not handle races between interruptJs and the checkForInterrupt functions - those must be + handled by the client of this class */ extern class KillCurrentOp { - enum { Off, On, All } state; - AtomicUInt toKill; + volatile bool _globalKill; public: - // the kill functions are not called with a mutex - void killAll() { state = All; interruptJs( 0 ); } - void kill(AtomicUInt i) { toKill = i; state = On; interruptJs( &i ); } - - // this is called without a mutex also, which means we can miss some - // kill requests; it's no worse than what the code was doing before, - // though, so I don't feel too bad about adding this function. - // hopefully we will have time for SERVER-1816 to fix this - void finishOp() { - if( state == On && cc().curop()->opNum() == toKill ) { - state = Off; + void killAll() { + _globalKill = true; + interruptJs( 0 ); + } + void kill(AtomicUInt i) { + { + scoped_lock l( Client::clientsMutex ); + for( set< Client* >::const_iterator j = Client::clients.begin(); j != Client::clients.end(); ++j ) { + if ( ( *j )->curop()->opNum() == i ) { + ( *j )->curop()->kill(); + break; + } + } } + interruptJs( &i ); } - void checkForInterrupt() { - if( state != Off ) { - if( state == All ) { - uasserted(11600,"interrupted at shutdown"); - } - if( cc().curop()->opNum() == toKill ) { - uasserted(11601,"interrupted"); - } + void checkForInterrupt() { + if( _globalKill ) { + uasserted(11600,"interrupted at shutdown"); + } + if( cc().curop()->killed() ) { + uasserted(11601,"interrupted"); } } const char *checkForInterruptNoAssert() { - if( state != Off ) { - if( state == All ) - return "interrupted at shutdown"; - if( cc().curop()->opNum() == toKill ) { - return "interrupted"; - } + if( _globalKill ) { + return "interrupted at shutdown"; + } + if( cc().curop()->killed() ) { + return "interrupted"; } return ""; } + private: void interruptJs( AtomicUInt *op ) { if ( !globalScriptEngine ) { return; diff --git a/db/instance.cpp b/db/instance.cpp index fdce160e949..6968b970b20 100644 --- a/db/instance.cpp +++ b/db/instance.cpp @@ -340,7 +340,6 @@ namespace mongo { } currentOp.ensureStarted(); currentOp.done(); - killCurrentOp.finishOp(); int ms = currentOp.totalTimeMillis(); log = log || (logLevel >= 2 && ++ctr % 512 == 0); diff --git a/jstests/killop.js b/jstests/killop.js new file mode 100644 index 00000000000..463292e63d2 --- /dev/null +++ b/jstests/killop.js @@ -0,0 +1,38 @@ +t = db.jstests_killop +t.drop(); + +function debug( x ) { + printjson( x ); +} + +t.save( {} ); +db.getLastError(); + +function ops() { + p = db.currentOp().inprog; + debug( p ); + ids = []; + for ( var i in p ) { + var o = p[ i ]; + if ( o.active && o.query && o.query.query && o.query.query.$where && o.ns == "test.jstests_killop" ) { + ids.push( o.opid ); + } + } + return ids; +} + +s1 = startParallelShell( "db.jstests_killop.count( { $where: function() { while( 1 ) { ; } } } )" ); +s2 = startParallelShell( "db.jstests_killop.count( { $where: function() { while( 1 ) { ; } } } )" ); + +o = []; +assert.soon( function() { o = ops(); return o.length == 2; } ); +db.killOp( o[ 0 ] ); +db.killOp( o[ 1 ] ); + +start = new Date(); + +s1(); +s2(); + +// don't want to pass if timeout killed the js function +assert( ( new Date() ) - start < 30000 ); diff --git a/mongo.xcodeproj/project.pbxproj b/mongo.xcodeproj/project.pbxproj index bc152affc56..cf962caafac 100644 --- a/mongo.xcodeproj/project.pbxproj +++ b/mongo.xcodeproj/project.pbxproj @@ -70,6 +70,8 @@ 932D855311AB912B002749FB /* fm4.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = fm4.js; sourceTree = ""; }; 932D855411AB912B002749FB /* geo_box1.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = geo_box1.js; sourceTree = ""; }; 93386F98125A7BB4005E0172 /* evale.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = evale.js; sourceTree = ""; }; + 93386FDA125A9357005E0172 /* drop2.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = drop2.js; sourceTree = ""; }; + 93386FDB125A9661005E0172 /* killop.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = killop.js; sourceTree = ""; }; 933A4D130F55A68600145C4B /* authTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = authTest.cpp; sourceTree = ""; }; 933A4D150F55A68600145C4B /* clientTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = clientTest.cpp; sourceTree = ""; }; 933A4D170F55A68600145C4B /* first.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = first.cpp; sourceTree = ""; }; @@ -842,6 +844,8 @@ 934BEB9A10DFFA9600178102 /* jstests */ = { isa = PBXGroup; children = ( + 93386FDB125A9661005E0172 /* killop.js */, + 93386FDA125A9357005E0172 /* drop2.js */, 93386F98125A7BB4005E0172 /* evale.js */, 936D5EC71251BA2A0015722C /* rename4.js */, 93B771FE124024A4007C8F0C /* evald.js */,