rufuscoder / Shakespeer

A Direct Connect Client for Multiple Platforms
GNU General Public License v2.0
28 stars 13 forks source link

sphubd share_tth.c function handle_tth_available_notification bug #157

Open rufuscoder opened 11 years ago

rufuscoder commented 11 years ago

Original author: zintis.p...@e-mail.lv (July 14, 2009 12:48:29)

What steps will reproduce the problem?

I'm sharing DVD image with all those .IFO .VOB files. sphubd log in debug mode shows some records like:

Tue 14 15:19:58 2816 <- (fd 12) add- hash$/home/Downloads/Mayday-The DVD-The.years.1991-2005.2DVD.2006.DVD- 9/MayDay The 2DVD/CD1/VIDEO_TS/VTS_15_0.IFO$GDVSWOZ7EDSSMPSXECKMBGD7X3R5R6VC5RRMYGY Tue 14 15:19:58 2816 -> (fd 14) status- message$$finished hashing VTS_15_0.IFO (0.31 MiB/s)|

Tue 14 15:19:58 2816 <- (fd 12) add- hash$/home/Downloads/Mayday-The DVD-The.years.1991-2005.2DVD.2006.DVD- 9/MayDay The 2DVD/CD1/VIDEO_TS/VTS_15_0.BUP$GDVSWOZ7EDSSMPSXECKMBGD7X3R5R6VC5RRMYGY Tue 14 15:19:58 2816 -> (fd 14) status- message$$finished hashing (0.31 MiB/s)|

I was wondering why the second message has no filename:

Tue 14 15:19:58 2816 -> (fd 14) status- message$$finished hashing (0.31 MiB/s)|

What is the expected output? What do you see instead?

there should be:

Tue 14 15:19:58 2816 -> (fd 14) status- message$$finished hashing VTS_15_0.BUP (0.31 MiB/s)|

the probleem looks like with function "handle_tth_available_notification bug" code:

struct tth_entry *te = tth_store_lookup(global_tth_store, notification- >tth);

with returns that file "VTS_15_0" is already shared because it founds VTS_15_0.IFO and te is not NULL. You can check it by

DEBUG("%s", original_file->partial_path);

What version of the product are you using? On what operating system?

Linux, shakespeer 0.9.11 and console client.

Original issue: http://code.google.com/p/shakespeer/issues/detail?id=132

rufuscoder commented 11 years ago

From zintis.p...@e-mail.lv on July 14, 2009 13:13:50 I found out that IFO and BUP files by md5 checksum are the same :( but the filenames are different and I believe we need them both. so the bus is not a thechnical but ideological.

rufuscoder commented 11 years ago

From zintis.p...@e-mail.lv on July 14, 2009 14:17:06 patch that fixes the issue.

rufuscoder commented 11 years ago

From zintis.p...@e-mail.lv on July 14, 2009 15:29:56 isuue was reported on forum: http://shakespeer.bzero.se/forum/viewtopic.php?id=1755

rufuscoder commented 11 years ago

From hwa...@gmail.com on July 14, 2009 15:35:24 Makes sense, thanks Zintis!

rufuscoder commented 11 years ago

From hwa...@gmail.com on July 14, 2009 15:44:25 However, all hashed files seem to be stored based on TTH, so I'm not sure what consequences adding a duplicate TTH will have; will it overwrite the last one?

See tthdb.c line 397 (tth_store_lookup)

Martin?

rufuscoder commented 11 years ago

From zintis.p...@e-mail.lv on July 14, 2009 18:12:19 I cant comment what impact this issue makes:( All I can say is that the patch allows to see all DVD files.

Another issue I have is that sphashd sometimes crashes. Not sure why. Researching.

rufuscoder commented 11 years ago

From martin.h...@gmail.com on July 15, 2009 06:39:03 The patch effectively disables rejecting duplicates from the share. Only files with the same TTH and the same path within a share are considered.

This defeats the whole point of duplicates. On the other hand, maybe it's the idea that is flawed... What is needed to properly support this is some sort of alias or soft link within a share, that doesn't affect the total share size (since that seems so important to hub owners).

The result is that you will end up with two or more files with the same TTH in your share. Requests based on TTH will serve the last scanned file. For uploading the content will be the same. In searches, the last scanned file's name will be returned.

Some background: The tth database has two indices: the primary is TTH and points to a tth_entry. This struct has an active_inode that specifies which actual file is shared, if there are duplicates. The second index is on inode (actually a comination of inode and size, see SHARE_STAT_TO_INODE) and maps inodes to TTHs. You can thus have multiple files (inodes) with the same TTH (ie, duplicates), but only one active inode that uses a TTH. If a share is removed, another inode in another share can claim the TTH and become the active inode for it.

Regarding the original bug about the filename not appearing in the status message, that is probably a segfault waiting to happen. The TTH available notification is handled both by share_tth.c and in ui_list.c. If there is a duplicate in share_tth.c, the notification->file struct will be freed. When the same notification is handled in ui_list.c, the file struct points to freed memory. A quick fix is to make sure ui_list.c handles the notification before share_tth.c.

HTH -martin