Re: LuaTask needs your help

[Thomas Lauer <thomas.lauer@...>]

Daniel Quintela <dq@soongsoft.com> wrote:
> 
> I'm almost ready to release LuaTask 1.6.2
> 
> I changed the way TASK_ENTRY memory is allocated.
> 
> I would appreciate it very much if you could test it and give me some 
> feedback.

This is too complicated, methinks. The old, simpler allocation scheme
was perfectly okay. The crashes resulted from invalid accesses, I think.
And I am not sure that these changes will solve all problems with that.

Following Kacper's clever analysis, I went through the ltask code step
by step. I finally changed just a few lines in taskthread(). This
produced a version that seems to run okay on my box. (I say "seems"
because the crashes I got were not strictly reproducible. I did
stress-test the code for a good while but with this sort of bug there's
always a small possibility that it's buggy but just refuses to crash.)

I didn't post these changes because I did other, unrelated changes as
well, so I couldn't just post a straightforward patch file. Also, I knew
you were already working on a fix anyway.

FWIW, the gist of my changes is the following. In the 1.6.1 version of
ltask.c, line 540, you initialise te:

te = aTask + ( ( long) vp);

The mutex is given up in line 554. After this line, the block aTask
points to can move at any time because of an realloc(). And if aTask
moves, then te is pointing nowhere.

However, in lines 558, 560, 567, 568, 571 ,573 and 583 te is still used.
(Getting the mutex back in 577 doesn't reinitialise te.)

I assume this is what produced the crashes I saw. When I changed the
code such that te is either not needed after line 554 or reinitialised
before accessing thread information everything seems to work fine (with
the caveat given).

Here is my current taskthread(). As I said, there are a few other
changes and I also compile this as C++ code. The three main changes are
commented:

static OS_THREAD_FUNC taskthread(void *vp) {
  TASK_ENTRY *te;
  int status=0;
  const char *init=getenv("LUA_INIT");
  OsSetThreadData(threadDataKey,vp);
  OsLockMutex(tlMutex,INFINITE);
  te=aTask+(long)vp;
#ifndef _WIN32
  pthread_setcanceltype( PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
  pthread_cleanup_push( taskCleanup, te);
#endif
  lua_State *L=te->L;        // <-- save te->L before giving up mutex
  const char *fn=te->fname;  // <-- save te->fname as well
  lua_gc(L,LUA_GCSTOP,0);
  luaL_openlibs(L);
  initLTask(L);
  lua_gc(L,LUA_GCRESTART, 0);
  OsUnlockMutex(tlMutex);
  if (init!=NULL) {
    if (init[0]=='@') status=dofile(L,init+1);
    else status=dostring(L,init,"=LUA_INIT");
  }
  if (status==0) {
    if (fn[0]=='@') dofile(L,fn+1);
    else dostring(L,fn,"=STRING_TASK");
  }
  OsLockMutex(tlMutex,INFINITE);
  te=aTask+(long)vp;  // <-- reinitialise te
#ifndef _WIN32
  pthread_cleanup_pop(0);
#endif
  taskCleanup(te);
  OsUnlockMutex(tlMutex);
  return 0;
}

Sorry, probably should have posted this earlier. Anyway, HTH.

-- 
cheers  thomasl

web : http://thomaslauer.com/start