From 4405730ee899f0a96d45eda019703118333cf2de Mon Sep 17 00:00:00 2001 From: Edward Wang Date: Mon, 31 Jan 2011 20:56:45 +0800 Subject: [PATCH 1/2] Comment on why both binary and integer infohash. Also part of the fix to issue 77, a.k.a refactor the code to identify torrent by its infohash instead of file path. --- apps/etorrent/include/types.hrl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/etorrent/include/types.hrl b/apps/etorrent/include/types.hrl index 518f16d6..4f8cc56a 100644 --- a/apps/etorrent/include/types.hrl +++ b/apps/etorrent/include/types.hrl @@ -34,6 +34,12 @@ % Types used by the DHT subsystem -type ipaddr() :: {byte(), byte(), byte(), byte()}. -type portnum() :: 1..16#FFFF. +% Currently there are two flavors of info hash used in etorrent +% code. One is a 160-bit binary produced by crypto:sha/1. The other +% is an integer decoded from such binary, which is used exclusively +% by DHT. DHT subsystem also uses it as node id and computes nodes +% distance by 'xor' their ids. Xor can not be applied on binaries. +% Thus the distinction. -type infohash() :: pos_integer(). -type nodeid() :: pos_integer(). -type nodeinfo() :: {nodeid(), ipaddr(), portnum()}. From 1388c15d2ccfc2c8b9ee2cb4efa1b052cf312d64 Mon Sep 17 00:00:00 2001 From: Edward Wang Date: Tue, 1 Feb 2011 20:07:59 +0800 Subject: [PATCH 2/2] Refactor to parse the raw torrent file only once. Now etorrent parses the raw torrent file only once in module etorrent_ctl. In order to do that, also chande the identification of torrent from its file path to info hash. For more information, please also check out the discussion in issue 77. --- apps/etorrent/src/etorrent_ctl.erl | 50 ++++++++++++++------- apps/etorrent/src/etorrent_fs_checker.erl | 24 +++------- apps/etorrent/src/etorrent_io_sup.erl | 15 +++---- apps/etorrent/src/etorrent_table.erl | 17 ++++--- apps/etorrent/src/etorrent_torrent_ctl.erl | 33 +++++++------- apps/etorrent/src/etorrent_torrent_pool.erl | 24 +++++----- apps/etorrent/src/etorrent_torrent_sup.erl | 14 +++--- 7 files changed, 95 insertions(+), 82 deletions(-) diff --git a/apps/etorrent/src/etorrent_ctl.erl b/apps/etorrent/src/etorrent_ctl.erl index 9a57783e..4c168451 100644 --- a/apps/etorrent/src/etorrent_ctl.erl +++ b/apps/etorrent/src/etorrent_ctl.erl @@ -9,6 +9,7 @@ -module(etorrent_ctl). -behaviour(gen_server). +-include("types.hrl"). -include("log.hrl"). -export([start_link/1, @@ -64,18 +65,21 @@ init([PeerId]) -> %% @private handle_cast({start, F}, S) -> ?INFO([starting, F]), - case torrent_duplicate(F) of - true -> {noreply, S}; - false -> - case etorrent_torrent_pool:start_child(F, S#state.local_peer_id, - etorrent_counters:next(torrent)) of + case load_torrent(F) of + duplicate -> {noreply, S}; + {ok, Torrent} -> + TorrentIH = etorrent_metainfo:get_infohash(Torrent), + case etorrent_torrent_pool:start_child( + {Torrent, F, TorrentIH}, + S#state.local_peer_id, + etorrent_counters:next(torrent)) of {ok, _} -> {noreply, S}; - {error, {already_started, _Pid}} -> {noreply, S}; - {error, Reason} -> - ?INFO([starting_error, Reason]), - etorrent_event:notify({starting_error, Reason}), - {noreply, S} - end + {error, {already_started, _Pid}} -> {noreply, S} + end; + {error, _Reason} -> + ?INFO([malformed_torrent_file, F]), + etorrent_event:notify({malformed_torrent_file, F}), + {noreply, S} end; handle_cast({check, Id}, S) -> Child = gproc:lookup_local_name({torrent, Id, control}), @@ -111,8 +115,9 @@ stop_torrent(F) -> ?INFO([stopping, F]), case etorrent_table:get_torrent({filename, F}) of not_found -> ok; % Was already removed, it is ok. - {value, _PL} -> - etorrent_torrent_pool:terminate_child(F), + {value, PL} -> + TorrentIH = proplists:get_value(info_hash, PL), + etorrent_torrent_pool:terminate_child(TorrentIH), ok end. @@ -123,10 +128,21 @@ stop_all() -> stop_torrent(F) end || PL <- PLS]. -torrent_duplicate(F) -> +-spec load_torrent(string()) -> duplicate + | {ok, bcode()} + | {error, _Reason}. +load_torrent(F) -> case etorrent_table:get_torrent({filename, F}) of - not_found -> false; - {value, PL} -> - duplicate =:= proplists:get_value(state, PL) + not_found -> load_torrent_internal(F); + {value, PL} -> + case duplicate =:= proplists:get_value(state, PL) of + true -> duplicate; + false -> load_torrent_internal(F) + end end. +load_torrent_internal(F) -> + Workdir = etorrent_config:work_dir(), + P = filename:join([Workdir, F]), + etorrent_bcoding:parse_file(P). + diff --git a/apps/etorrent/src/etorrent_fs_checker.erl b/apps/etorrent/src/etorrent_fs_checker.erl index c8961367..8cf25157 100644 --- a/apps/etorrent/src/etorrent_fs_checker.erl +++ b/apps/etorrent/src/etorrent_fs_checker.erl @@ -29,15 +29,14 @@ check_torrent_for_bad_pieces(Id) -> end]. % @doc Read and check a torrent -%

The torrent given by Id, at Path (the .torrent file) will be checked for +%

The torrent given by Id, and parsed content will be checked for % correctness. We return a tuple % with various information about said torrent: The decoded Torrent dictionary, % the info hash and the number of pieces in the torrent.

% @end --spec read_and_check_torrent(integer(), string()) -> {ok, bcode(), binary(), integer()}. -read_and_check_torrent(Id, Path) -> - {ok, Torrent, Infohash, Hashes} = - initialize_dictionary(Id, Path), +-spec read_and_check_torrent(integer(), bcode()) -> {ok, integer()}. +read_and_check_torrent(Id, Torrent) -> + {ok, Hashes} = initialize_dictionary(Id, Torrent), L = length(Hashes), @@ -63,7 +62,7 @@ read_and_check_torrent(Id, Path) -> ok = initialize_pieces_from_disk(FS, Id, Hashes) end, - {ok, Torrent, Infohash, L}. + {ok, L}. %% @doc Check a piece for completion and mark it for correctness %% @end @@ -97,19 +96,10 @@ check_piece(TorrentID, PieceIndex) -> %% ======================================================================= -initialize_dictionary(Id, Path) -> - %% Load the torrent - {ok, Torrent, IH} = load_torrent(Path), +initialize_dictionary(Id, Torrent) -> ok = etorrent_io:allocate(Id), Hashes = etorrent_metainfo:get_pieces(Torrent), - {ok, Torrent, IH, Hashes}. - -load_torrent(Path) -> - Workdir = etorrent_config:work_dir(), - P = filename:join([Workdir, Path]), - {ok, Torrent} = etorrent_bcoding:parse_file(P), - InfoHash = etorrent_metainfo:get_infohash(Torrent), - {ok, Torrent, InfoHash}. + {ok, Hashes}. initialize_pieces_seed(Id, Hashes) -> L = length(Hashes), diff --git a/apps/etorrent/src/etorrent_io_sup.erl b/apps/etorrent/src/etorrent_io_sup.erl index 44674a1d..1e6b7f1a 100644 --- a/apps/etorrent/src/etorrent_io_sup.erl +++ b/apps/etorrent/src/etorrent_io_sup.erl @@ -11,20 +11,17 @@ -export([init/1]). %% @doc Initiate the supervisor. -%%

The arguments are the ID of the torrent and the file-path at -%% which the torrent file lives

+%%

The arguments are the ID of the torrent and the +%% parsed torrent file

%% @end --spec start_link(torrent_id(), file_path()) -> {'ok', pid()}. -start_link(TorrentID, TorrentFile) -> - supervisor:start_link(?MODULE, [TorrentID, TorrentFile]). +-spec start_link(torrent_id(), bcode()) -> {'ok', pid()}. +start_link(TorrentID, Torrent) -> + supervisor:start_link(?MODULE, [TorrentID, Torrent]). %% ---------------------------------------------------------------------- %% @private -init([TorrentID, TorrentFile]) -> - Workdir = etorrent_config:work_dir(), - FullPath = filename:join([Workdir, TorrentFile]), - {ok, Torrent} = etorrent_bcoding:parse_file(FullPath), +init([TorrentID, Torrent]) -> Files = etorrent_metainfo:file_paths(Torrent), DirServer = directory_server_spec(TorrentID, Torrent), Dldir = etorrent_config:download_dir(), diff --git a/apps/etorrent/src/etorrent_table.erl b/apps/etorrent/src/etorrent_table.erl index 3e2bd6c0..83406880 100644 --- a/apps/etorrent/src/etorrent_table.erl +++ b/apps/etorrent/src/etorrent_table.erl @@ -25,7 +25,7 @@ %% Torrent information -export([all_torrents/0, statechange_torrent/2, get_torrent/1, acquire_check_token/1, - new_torrent/3]). + new_torrent/4]). %% gen_server callbacks -export([init/1, handle_call/3, handle_cast/2, handle_info/2, @@ -47,7 +47,7 @@ -record(tracking_map, {id :: '_' | integer(), %% Unique identifier of torrent filename :: '_' | string(), %% The filename supervisor_pid :: '_' | pid(), %% The Pid of who is supervising - info_hash :: '_' | binary() | 'unknown', + info_hash :: '_' | binary(), state :: '_' | tracking_map_state()}). @@ -184,16 +184,19 @@ new_peer(IP, Port, TorrentId, Pid, State) -> add_monitor(peer, Pid). %% @doc Add a new torrent -%%

The torrent is given by File with the Supervisor pid as given to the -%% database structure.

+%%

The torrent is given by File and its infohash with the +%% Supervisor pid as given to the database structure.

%% @end --spec new_torrent(string(), pid(), integer()) -> ok. -new_torrent(File, Supervisor, Id) when is_integer(Id), is_pid(Supervisor), is_list(File) -> +-spec new_torrent(string(), binary(), pid(), integer()) -> ok. +new_torrent(File, IH, Supervisor, Id) when is_integer(Id), + is_pid(Supervisor), + is_binary(IH), + is_list(File) -> add_monitor({torrent, Id}, Supervisor), TM = #tracking_map { id = Id, filename = File, supervisor_pid = Supervisor, - info_hash = unknown, + info_hash = IH, state = awaiting}, true = ets:insert(tracking_map, TM), ok. diff --git a/apps/etorrent/src/etorrent_torrent_ctl.erl b/apps/etorrent/src/etorrent_torrent_ctl.erl index 6b4798f5..df168e3d 100644 --- a/apps/etorrent/src/etorrent_torrent_ctl.erl +++ b/apps/etorrent/src/etorrent_torrent_ctl.erl @@ -14,6 +14,7 @@ -behaviour(gen_fsm). -include("log.hrl"). +-include("types.hrl"). -ignore_xref([{'start_link', 3}, {start, 1}, {initializing, 2}, {started, 2}, {stopped, 2}, {stop, 1}]). @@ -30,7 +31,8 @@ code_change/4]). -record(state, {id :: integer() , - path :: string(), + torrent :: bcode(), % Parsed torrent file + info_hash :: binary(), % Infohash of torrent file peer_id :: binary(), parent_pid :: pid(), tracker_pid :: pid() }). @@ -40,10 +42,10 @@ %% ==================================================================== %% @doc Start the server process --spec start_link(integer(), string(), binary()) -> +-spec start_link(integer(), {bcode(), string(), binary()}, binary()) -> {ok, pid()} | ignore | {error, term()}. -start_link(Id, Path, PeerId) -> - gen_fsm:start_link(?MODULE, [self(), Id, Path, PeerId], []). +start_link(Id, {Torrent, TorrentFile, TorrentIH}, PeerId) -> + gen_fsm:start_link(?MODULE, [self(), Id, {Torrent, TorrentFile, TorrentIH}, PeerId], []). %% @doc Request that the given torrent is stopped %% @end @@ -84,12 +86,13 @@ completed(Pid) -> %% ==================================================================== %% @private -init([Parent, Id, Path, PeerId]) -> - etorrent_table:new_torrent(Path, Parent, Id), +init([Parent, Id, {Torrent, TorrentFile, TorrentIH}, PeerId]) -> + etorrent_table:new_torrent(TorrentFile, TorrentIH, Parent, Id), etorrent_chunk_mgr:new(Id), gproc:add_local_name({torrent, Id, control}), {ok, initializing, #state{id = Id, - path = Path, + torrent = Torrent, + info_hash = TorrentIH, peer_id = PeerId, parent_pid = Parent}, 0}. % Force timeout instantly. @@ -104,13 +107,11 @@ initializing(timeout, S) -> %% Read the torrent, check its contents for what we are missing etorrent_event:checking_torrent(S#state.id), - {ok, Torrent, InfoHash, NumberOfPieces} = + {ok, NumberOfPieces} = etorrent_fs_checker:read_and_check_torrent(S#state.id, - S#state.path), + S#state.torrent), etorrent_piece_mgr:add_monitor(self(), S#state.id), - %% Update the tracking map. This torrent has been started, and we - %% know its infohash - etorrent_table:statechange_torrent(S#state.id, {infohash, InfoHash}), + %% Update the tracking map. This torrent has been started. etorrent_table:statechange_torrent(S#state.id, started), {AU, AD} = @@ -127,16 +128,16 @@ initializing(timeout, S) -> {downloaded, 0}, {all_time_uploaded, AU}, {all_time_downloaded, AD}, - {left, calculate_amount_left(S#state.id, NumberOfPieces, Torrent)}, - {total, etorrent_metainfo:get_length(Torrent)}}, + {left, calculate_amount_left(S#state.id, NumberOfPieces, S#state.torrent)}, + {total, etorrent_metainfo:get_length(S#state.torrent)}}, NumberOfPieces), %% Start the tracker {ok, TrackerPid} = etorrent_torrent_sup:add_tracker( S#state.parent_pid, - etorrent_metainfo:get_url(Torrent), - etorrent_metainfo:get_infohash(Torrent), + etorrent_metainfo:get_url(S#state.torrent), + S#state.info_hash, S#state.peer_id, S#state.id), diff --git a/apps/etorrent/src/etorrent_torrent_pool.erl b/apps/etorrent/src/etorrent_torrent_pool.erl index e771f55f..a4283713 100644 --- a/apps/etorrent/src/etorrent_torrent_pool.erl +++ b/apps/etorrent/src/etorrent_torrent_pool.erl @@ -5,6 +5,8 @@ -behaviour(supervisor). +-include("types.hrl"). + %% API -export([start_link/0, start_child/3, terminate_child/1]). @@ -19,23 +21,25 @@ start_link() -> supervisor:start_link({local, ?SERVER}, ?MODULE, []). % @doc Add a new torrent file to the system -%

The torrent file is given by File. Our PeerId is also given, as well as +%

The torrent file is given by its bcode representation, file name +% and info hash. Our PeerId is also given, as well as % the Id we wish to use for that torrent.

% @end --spec start_child(string(), binary(), integer()) -> +-spec start_child({bcode(), string(), binary()}, binary(), integer()) -> {ok, pid()} | {ok, pid(), term()} | {error, term()}. -start_child(File, Local_PeerId, Id) -> - ChildSpec = {File, - {etorrent_torrent_sup, start_link, [File, Local_PeerId, Id]}, +start_child({Torrent, TorrentFile, TorrentIH}, Local_PeerId, Id) -> + ChildSpec = {TorrentIH, + {etorrent_torrent_sup, start_link, + [{Torrent, TorrentFile, TorrentIH}, Local_PeerId, Id]}, transient, infinity, supervisor, [etorrent_torrent_sup]}, supervisor:start_child(?SERVER, ChildSpec). -% @doc Ask to stop the torrent represented by File. +% @doc Ask to stop the torrent represented by its info_hash. % @end --spec terminate_child(string()) -> ok. -terminate_child(File) -> - supervisor:terminate_child(?SERVER, File), - supervisor:delete_child(?SERVER, File). +-spec terminate_child(binary()) -> ok. +terminate_child(TorrentIH) -> + supervisor:terminate_child(?SERVER, TorrentIH), + supervisor:delete_child(?SERVER, TorrentIH). %% ==================================================================== diff --git a/apps/etorrent/src/etorrent_torrent_sup.erl b/apps/etorrent/src/etorrent_torrent_sup.erl index b8912184..bb138299 100644 --- a/apps/etorrent/src/etorrent_torrent_sup.erl +++ b/apps/etorrent/src/etorrent_torrent_sup.erl @@ -19,9 +19,10 @@ %% @doc Start up the supervisor %% @end --spec start_link(string(), binary(), integer()) -> {ok, pid()} | ignore | {error, term()}. -start_link(File, Local_PeerId, Id) -> - supervisor:start_link(?MODULE, [File, Local_PeerId, Id]). +-spec start_link({bcode(), string(), binary()}, binary(), integer()) -> + {ok, pid()} | ignore | {error, term()}. +start_link({Torrent, TorrentFile, TorrentIH}, Local_PeerId, Id) -> + supervisor:start_link(?MODULE, [{Torrent, TorrentFile, TorrentIH}, Local_PeerId, Id]). %% @doc Add the tracker process to the supervisor %%

We do this after-the-fact as we like to make sure how complete the torrent @@ -60,15 +61,16 @@ add_peer(PeerId, InfoHash, TorrentId, {IP, Port}, Capabilities, Socket) -> %% ==================================================================== %% @private -init([Path, PeerId, Id]) -> +init([{Torrent, TorrentFile, TorrentIH}, PeerId, Id]) -> FSPool = {fs_pool, - {etorrent_io_sup, start_link, [Id, Path]}, + {etorrent_io_sup, start_link, [Id, Torrent]}, transient, infinity, supervisor, [etorrent_io_sup]}, %FS = {fs, % {etorrent_fs, start_link, [Id]}, % permanent, 2000, worker, [etorrent_fs]}, Control = {control, - {etorrent_torrent_ctl, start_link, [Id, Path, PeerId]}, + {etorrent_torrent_ctl, start_link, + [Id, {Torrent, TorrentFile, TorrentIH}, PeerId]}, permanent, 20000, worker, [etorrent_torrent_ctl]}, PeerPool = {peer_pool_sup, {etorrent_peer_pool, start_link, [Id]},