Now we can't run more than one DumpRenderTree on one
machine, because DRT processes use same temporary
directory for local storage, icon database, etc.
(Or one DRT per user on Mac because of user temp.)
Running more DRT on one machine is useful if more then
one people (or buildbot) work on a powerful multicore
server, or if you would like to make run-webkit-test
script parallel to decrease runtime of layouttesting.
Created attachment 52199 [details]
This patch tested and works on Qt-linux and on Mac-Leopard.
Now the temporary directory for local storage and icon db is
~/.local on Qt, it will be $TMP/DumpRenderTree-XXXXXX with this patch.
On Leopard it was ~/Library/Application Support/DumpRenderTree
and it will be $TMP/DumpRenderTree-XXXXXX (here it was user temp)
This patch would solve many strange storage, workers crashes on Qt bots.
I don't think the Mac OS X change is correct. There are a lot more places than this that use the home directory. It is better just to change the home directory for that platform rather than use this other environment variable.
(In reply to comment #2)
I think Mac DumpRenderTree only use home directory in this place.
I can't find other places, but it is possible there are.
As far as I know, now all database can be found here:
I consider it isn't a good idea to change home directory,
I think it is an ugly hack. And I'm not sure if it works.
There are some other files in user temp, but
it is an other problem what we should fix:
(On Mac now it is /var/folders/zz/zz7dAsWSGUu8b-4hUGMKs++++TY/-Tmp-
On Qt-Linux there isn't user temp, there is only the global /tmp)
Comment on attachment 52199 [details]
> + return [NSString stringWithCString: getenv("DUMPRENDERTREE_TEMP")];
The correct function for this is:
The stringWithCString function will not work properly for non-ASCII characters.
Created attachment 52261 [details]
Mac code is fixed by the advice of Darin.
Anybody for review, please?
Comment on attachment 52261 [details]
I don't believe that this is sufficient to make Mac work, but it is probably better than what we have now.
Also, why call getenv so many times? Why not just once and cache the result in a local variable?
Do we know if this makes the IconDatbase crashers on the Qt bot go away?
(In reply to comment #7)
> (From update of attachment 52261 [details])
> I don't believe that this is sufficient to make Mac work, but it is probably
> better than what we have now.
This patch isn't enough to make Mac work, but it is
necessary to avoid concurent database file accessses.
We have ideas to fix other problems mentioned here and
in the master bug. But I don't want to upload a very big
patch for review. I prefer one patch for one separated fix.
> Also, why call getenv so many times? Why not just once and cache the result in
> a local variable?
You're right, it isn't so nice, I'll fix. New patch is coming soon.
> Do we know if this makes the IconDatbase crashers on the Qt bot go away?
I'm not sure enough, but I hope yes. I didn't want to apply this patch
for testing on the official bot before landing, because I'm afraid of svn
conflict and following fresh build. :))
Created attachment 52726 [details]
use local variable instead of calling getenv many times
(In reply to comment #9)
> Created an attachment (id=52726) [details]
> updated patch
> use local variable instead of calling getenv many times
One more reaseon why this patch is useful:
(We really need it to avoid false positive alarms)
Comment on attachment 52726 [details]
Stylistically this is wrong. variable names are wrong, spacing is wrong. You changed only one caller of getenv.
This is an incomplete hack. It's a step in the right direction, but not a full fix.
Created attachment 52942 [details]
Created attachment 52943 [details]
Comment on attachment 52943 [details]
OK. I look forward to hearing good things from the Qt bot.
(In reply to comment #13)
> Created an attachment (id=52943) [details]
Landed in: http://trac.webkit.org/changeset/57328