On Fri, 20 Feb 2009 14:45:04 +1100, "James ." jazzrz86@gmail.com wrote:
Thanks for the patch. After some short time of debugging, I finally found the root cause of the problem. It is indeed a race condition, and the reason geany 0.15 did not catch this is because it duplicated all the tags in get_tag_list whereas now, all the tags come straight from the tag manager.
As it turns out, the tag manager makes some naive assumptions about when the file is updated and the time it analyze the tags:
from tm_work_object.c:
[snipped code]
as you can see, while tm_work_object_is_changed compares analyze_time with the file timestamp, the update function only feeds the CURRENT time into analyze_time after its updated, which opens the door for some nasty race conditions when dealing with remote files.
The solution, thus is simply to update analyze_time with the file timestamp:
-source_file->analyze_time = time(NULL); +source_file->analyze_time = tm_get_file_timestamp (source_file->file_name);
and bingo, problem disappear for good, no hacks.
Wow, many thanks for getting so deeply into the problem and actually finding the evil thing.
Yesterday, I had the tagmanager file changed code shortly in mind but then when writing the mail, I already forgot about it. But great it's now up again. I was wondering this already for a long time: I think we can completely drop the tm_work_object_is_changed() call in tm_source_file_update() because when we call tm_source_file_update() in Geany, we really want to reload the tags, e.g. at re-reading the symbol list after a file has saved. So, there is basically no need to keep the file time checks in tagmanager and by removing it we would also save a stat() call on the file which would be great especially for remote files.
I will have a deeper look into this next week and either commit your fix to use the correct time value or drop the whole code as mentioned above.
Thanks again.
Regards, Enrico