← Back to branch summary

~drizzle-trunk/drizzle/development

« back to all changes in this revision

Viewing changes to drizzled/sql_base.cc

  • Committer: Olaf van der Spek
  • Date: 2011-04-19 13:46:56 UTC
  • mto: This revision was merged to the branch mainline in revision 2288.
  • Revision ID: olafvdspek@gmail.com-20110419134656-pp22rdpim5o73oiu
Delete write-only Session::some_tables_deleted

Show diffs side-by-side

added added

removed removed

Lines of Context:
drizzled/sql_base.cc
177
177
 
178
178
  {
179
179
    boost::mutex::scoped_lock scopedLock(table::Cache::mutex()); /* Optionally lock for remove tables from open_cahe if not in use */
180
 
 
181
 
    if (tables == NULL)
 
180
    if (not tables)
182
181
    {
183
182
      g_refresh_version++;                              // Force close of open tables
184
 
 
185
183
      table::getUnused().clear();
186
 
 
187
 
      if (wait_for_refresh)
188
 
      {
189
 
        /*
190
 
          Other threads could wait in a loop in open_and_lock_tables(),
191
 
          trying to lock one or more of our tables.
192
 
 
193
 
          If they wait for the locks in thr_multi_lock(), their lock
194
 
          request is aborted. They loop in open_and_lock_tables() and
195
 
          enter open_table(). Here they notice the table is refreshed and
196
 
          wait for COND_refresh. Then they loop again in
197
 
          openTablesLock() and this time open_table() succeeds. At
198
 
          this moment, if we (the FLUSH TABLES thread) are scheduled and
199
 
          on another FLUSH TABLES enter close_cached_tables(), they could
200
 
          awake while we sleep below, waiting for others threads (us) to
201
 
          close their open tables. If this happens, the other threads
202
 
          would find the tables unlocked. They would get the locks, one
203
 
          after the other, and could do their destructive work. This is an
204
 
          issue if we have LOCK TABLES in effect.
205
 
 
206
 
          The problem is that the other threads passed all checks in
207
 
          open_table() before we refresh the table.
208
 
 
209
 
          The fix for this problem is to set some_tables_deleted for all
210
 
          threads with open tables. These threads can still get their
211
 
          locks, but will immediately release them again after checking
212
 
          this variable. They will then loop in openTablesLock()
213
 
          again. There they will wait until we update all tables version
214
 
          below.
215
 
 
216
 
          Setting some_tables_deleted is done by table::Cache::removeTable()
217
 
          in the other branch.
218
 
 
219
 
          In other words (reviewer suggestion): You need this setting of
220
 
          some_tables_deleted for the case when table was opened and all
221
 
          related checks were passed before incrementing refresh_version
222
 
          (which you already have) but attempt to lock the table happened
223
 
          after the call to Session::close_old_data_files() i.e. after removal of
224
 
          current thread locks.
225
 
        */
226
 
        BOOST_FOREACH(table::CacheMap::const_reference iter, table::getCache())
227
 
        {
228
 
          if (iter.second->in_use)
229
 
            iter.second->in_use->some_tables_deleted= false;
230
 
        }
231
 
      }
232
184
    }
233
185
    else
234
186
    {
387
339
  {
388
340
    found_old_table|= free_cached_table(scoped_lock);
389
341
  }
390
 
  some_tables_deleted= false;
391
342
 
392
343
  if (found_old_table)
393
344
  {
1278
1229
 
1279
1230
  if (tables != tables_ptr)                     // Should we get back old locks
1280
1231
  {
1281
 
    DrizzleLock *local_lock;
1282
1232
    /*
1283
1233
      We should always get these locks. Anyway, we must not go into
1284
1234
      wait_for_tables() as it tries to acquire table::Cache::mutex(), which is
1285
1235
      already locked.
1286
1236
    */
1287
 
    some_tables_deleted= false;
1288
1237
 
1289
 
    if ((local_lock= lockTables(tables, (uint32_t) (tables_ptr - tables), flags)))
1290
 
    {
1291
 
      /* unused */
1292
 
    }
1293
 
    else
 
1238
    if (not lockTables(tables, (uint32_t) (tables_ptr - tables), flags))
1294
1239
    {
1295
1240
      /*
1296
1241
        This case should only happen if there is a bug in the reopen logic.