Official eMule-Board: Emule 0.50B Beta1 Released - Official eMule-Board

Jump to content


  • (16 Pages)
  • +
  • « First
  • 14
  • 15
  • 16

Emule 0.50B Beta1 Released

#301 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 12 September 2018 - 09:16 PM

View PostEnig123, on 12 September 2018 - 02:22 AM, said:

Since version 0.50B Beta, eMule uses another thread to buffer uploading data.

Asynchronous I/O is used; it is quite different from ordinary synchronous operations.

View PostEnig123, on 12 September 2018 - 02:22 AM, said:

I have already encountered downloading data corruptions which may be caused by thread problems I mentioned above

Did it happen with the original 0.50b code?
0

#302 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 13 September 2018 - 12:51 AM

View Postfox88, on 12 September 2018 - 02:16 PM, said:

View PostEnig123, on 12 September 2018 - 02:22 AM, said:

I have already encountered downloading data corruptions which may be caused by thread problems I mentioned above

Did it happen with the original 0.50b code?


If it's caused by thread problem, then I believe 0.50b code is also vulnerable, even though it could be rare.
0

#303 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 13 September 2018 - 01:07 PM

There are very few places where files are read in UploadDiskIOThread.cpp.
Could you please describe when asynchronous reading would badly interfere with synchronous operations in plain 0.50b code?
0

#304 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 13 September 2018 - 05:09 PM

View Postfox88, on 13 September 2018 - 06:07 AM, said:

There are very few places where files are read in UploadDiskIOThread.cpp.
Could you please describe when asynchronous reading would badly interfere with synchronous operations in plain 0.50b code?


Is it possible that during the part file's buffer being flushed, the CUploadDiskIOThread started reading the same part file, which interferes with the internal current position of the file handle?
0

#305 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 13 September 2018 - 09:15 PM

View PostEnig123, on 13 September 2018 - 08:09 PM, said:

Is it possible that during the part file's buffer being flushed, the CUploadDiskIOThread started reading the same part file, which interferes with the internal current position of the file handle?

That just rephrases the original statement as a question.
But it was expected to get a detailed explanation: which methods, what line numbers, what conditions - and so on for 0.50b code.
This is not a huge request, because a problem must be clearly understood before getting fixed.
0

#306 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 13 September 2018 - 11:05 PM

View Postfox88, on 13 September 2018 - 02:15 PM, said:

That just rephrases the original statement as a question.
But it was expected to get a detailed explanation: which methods, what line numbers, what conditions - and so on for 0.50b code.
This is not a huge request, because a problem must be clearly understood before getting fixed.


In current code base of 0.50b, in CPartFile::FlushBuffer of PartFile.cpp, line 4825~4826, if the handle of the specific CPartFile is being manipulated by ReadFile call of line 316 in CUploadDiskIOThread.cpp, it's possible that the Write call in PartFile.cpp line 4826 will be written to a wrong position.

Edit: I suspect it may related to some unspeakable downloaded data corruption somehow.

Edit2: The experiments on my codebase gave more credibility on my above theory. By writing a series of consecutive downloaded buffer blocks as a whole to the disk in one loop, and leaving the costly memory release operations after the loop, the mysterious download corruptions here and there are completely vanished, at least in 1 day and 4 hours running eMule with heavy downloads (32 files finished).

It doesn't mean that the thread conflicts are gone, it just narrowed down the window that could cause the conflicts. So it's probably a real bug after introducing asynchronous uploading, and it can be fixed.

Edit3: Back to 0.50b codebase and refer to the correct line.

This post has been edited by Enig123: 14 September 2018 - 05:01 AM

0

#307 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 14 September 2018 - 07:30 AM

View PostEnig123, on 14 September 2018 - 02:05 AM, said:

it's possible that the Write call in PartFile.cpp line 4826 will be written to a wrong position.

Could you please try to make a more detailed analysis after reading at least this article?

View PostEnig123, on 14 September 2018 - 02:05 AM, said:

Edit2: The experiments on my codebase gave more credibility on my above theory.

Your codebase substantially differs from the official.
Let's find out if this corruption is possible at all in 0.50b.
0

#308 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 17 September 2018 - 12:59 AM

There's a bug with the asynchronous I/O regarding upload buffering, namely while a slot has being buffering with asynchronous ReadFile, before it's finished, if there're extra block requests, the CUploadDiskIOThread is capable of invoke StartCreateNextBlockPackage another time, since there's no check if any read operation is pending.

This will result in unnecessary consuming of block requests, and consequently using more memory. I have noticed the unusual disk read behavior with my mod for a while, in my mod the requests are capped to 3, instead of 5 as 0.50b does. It also affect 0.50b of course, since some high speed upload clients can send more than 5 requests.

The solution is introduce a counter in UploadingToClient_Struct, invoke StartCreateNextBlockPackage only when the counter is 0, which means no pending reading blocks.
0

#309 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 17 September 2018 - 09:48 AM

View PostEnig123, on 17 September 2018 - 03:59 AM, said:

It also affect 0.50b of course

There should be an early return from StartCreateNextBlockPackage if sufficient data was buffered.
0

#310 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 17 September 2018 - 12:24 PM

View Postfox88, on 17 September 2018 - 02:48 AM, said:

View PostEnig123, on 17 September 2018 - 03:59 AM, said:

It also affect 0.50b of course

There should be an early return from StartCreateNextBlockPackage if sufficient data was buffered.


Not before the former reads finished, at least for some of them.
0

#311 User is offline   fox88 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 4580
  • Joined: 13-May 07

Posted 18 September 2018 - 09:45 AM

View PostEnig123, on 17 September 2018 - 03:24 PM, said:

Not before the former reads finished, at least for some of them.

Data size for pending reads is also counted as buffered.
0

#312 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 502
  • Joined: 22-November 04

Posted 18 September 2018 - 06:49 PM

View Postfox88, on 18 September 2018 - 02:45 AM, said:

Data size for pending reads is also counted as buffered.


Thank you fox88. Now I am starting to understand the asynchronous I/O thing.
0

  • Member Options

  • (16 Pages)
  • +
  • « First
  • 14
  • 15
  • 16

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users