Scythe42's fixes for 1.4.0 (3 Viewers)

[Wo0zy]

I need to stop here. Anyone else can reproduce this problem?

Hi Scythe,

Yes. I have the problem as well. Only just realised. But I'm not currently running your latest build so will update, re-test and if the problem is still present I'll post logs.

Cheers,

Wo0zy

 

[edterbak]

Scythe.
I want to test for you/us this evening. Gatto catch the train.
Could you be clear on what you still need tested? Howto reproduce.. etc.

 

[Scythe42]

Regarding the locking: Using foreach with a dictionary in a public context is not thread safe. That is the nature of a Dictionary and how foreach uses it. As soon as a key is added or removed the foreach cannot continue properly. LINQ should show the same.

That is the only thing in TV-Series that requries for 1.4.0 as a quick and dirty fix. The rest should be fine as no other exceptions or issues were reported.

But coming to the defense here of TV-Series right away: It is not clear which other thread uses the same object. In theory could even be another plugin as the methods are public.

But by locking the foreach loop when going through the Dictionary, TV-Series itself will not cause an exception and can continue. And is is just for logging some information. The rest seems to run fine.

What needs to be done in the long run:

• Check if any other thread create by TV-Series modified the Dictionary during initialization
• Make these methods internal instead of public to avoid other plugins recycling them.

Dictionaries and thread safe programming are always a pain. They require a lot of locks as Dictionaries in C# are not thread safe.

Starting with .NET 4 MS introduces a thread safe dictionary exactly for that reason: http://msdn.microsoft.com/en-us/library/dd287191.aspx

 

[Scythe42]

3. Went into TV Section and started LiveTV (DRR changed to 50Hz) and stopped playback -> Rendering @ 50FPS!

Can you please give this MediaPortal.exe a quick try and let me know if this changes stuff?

If not I need to add some debug logging to the timer methods to see where a wrong or unexpected value is coming from.

 

[HomeY]

3. Went into TV Section and started LiveTV (DRR changed to 50Hz) and stopped playback -> Rendering @ 50FPS!

Can you please give this MediaPortal.exe a quick try and let me know if this changes stuff?

If not I need to add some debug logging to the timer methods to see where a wrong or unexpected value is coming from.

Won't start on HTPC:

[2013-04-27 14:06:39,231] [Error  ] [MPMain  ] [ERROR] - D3D: Could not create device

Hangs on initializing.

On DEV (your branch):

[2013-04-27 14:12:11,504] [Error  ] [MPMain  ] [ERROR] - D3D: Could not create device
[2013-04-27 14:12:19,593] [Log	] [MPMain  ] [ERROR] - Exception: System.NullReferenceException: Object reference not set to an instance of an object.
  at MediaPortal.D3D.InitializeDevice()
  at MediaPortal.D3D.Init()
  at MediaPortalApp.Main(String[] args)  Message: Object reference not set to an instance of an object.  Site  : Void InitializeDevice()  Source : MediaPortal  Stack Trace:	at MediaPortal.D3D.InitializeDevice()
  at MediaPortal.D3D.Init()
  at MediaPortalApp.Main(String[] args)
[2013-04-27 14:12:19,594] [Error  ] [MPMain  ] [ERROR] - MediaPortal stopped due to an exception Object reference not set to an instance of an object. MediaPortal	at MediaPortal.D3D.InitializeDevice()
  at MediaPortal.D3D.Init()
  at MediaPortalApp.Main(String[] args)

 

[SilentBob]

I'm not sure if this is correct. If the idea is to synchronize access to optionsCache, the following line does not work correctly

if (optionsCache.ContainsKey(convertedProperty)) return optionsCache[convertedProperty];

After the if-check optionsCache can be modified by another thread and if the item is removed the return will fail?

in principle you are right and this would be the full and clean solution. That's why i wrote

but, as I do not see any remove of complete KeyValue Pairs, you do not need to lock this one Code (text): publicstaticDBValueGetOptions(String property) ... if(optionsCache.ContainsKey(convertedProperty))return optionsCache[convertedProperty];

because in whole code the dictionary is just added or updated no deletion is done.
the only thing what could happen is that you read an old value, but the key will be always available.


Nevertheless see scythe post
https://forum.team-mediaportal.com/threads/scythe42s-fixes-for-1-4-0.118345/page-26#post-988762
For this .Net 4 would be needed, but also having these concurrent data types in place, multithreading is a powerful mechanism for which some preconditions have to be established in advance. Also senior developer sometimes feel the pain with it.

What should be fixed here is also just one thing of a long way getting all plugins thread-safe. There needs to be more guidance for plugin developer as well: what is allowed what not. Providing public APIs should always be thread-safe.
Especially the initialization is a critical phase, if a plugin1 is in initialization phase and not yet ready to be used, another plugin2 cannot use the public APIs, because plugin1 did not reach the "ready" state.

Threading issues are mostly timing issues, hard to reproduce and for an understanding you need to have the big picture (what is running in parallel, what are critical sections, which unexpected execution order can cause a problem)

That's why I also said in my last post, maybe it would be an idea to think about a controlled parallelization.
1. create a dependency graph of all available plugins
2. find the plugins which do just depend on native MediaPotal and not on other Plugins, they can all be initialized in parallel
3. find the plugins which depend on MediaPortal and the plugins initialized in previous step they can all be initialized in parallel
4. repeat 3rd step until all plugins are initialized

 

[seco]

Yes, in general multithreading is a tough subject. I think we haven't seen all the problems yet and we should do more testing, that's why I feel we should not include threaded plugin loading in MP version 1.4.0. This also was already discussed internally and probably the way it's going to be.

 

[FreakyJ]

1. create a dependency graph of all available plugins 2. find the plugins which do just depend on native MediaPotal and not on other Plugins, they can all be initialized in parallel 3. find the plugins which depend on MediaPortal and the plugins initialized in previous step they can all be initialized in parallel 4. repeat 3rd step until all plugins are initialized

this is done in MP2

 

[legnod]

Windowed and/or Fullscreen mode? Any other stuff like always on top checked? When using full screen: is minimize to tray on focus loss checked? Over the Menu Bar, the Top Bar of MP, over a Skin Menu, Alt F4 etc.

Closing MP does not work at all (doesnt depend if fullscreen/window) if "reduce MP to tray on exit" is enabled and action "97" is called from within the GUI to close MP. (this is used by topbar and some other menus within the GUI)

You do not want to see MP? Then just minimize the goddam window it is still one mouse click or button press or if in fullscreen activate minimization of loss of focus and it goes away when you switch context by whatever means.

Please dont get me wrong...i dont use this setting at all but this thread is for testing so i test everything available
I am totally on your side that MP has a lot of settings and features that maybe nobody really uses and makes it hard for the developers.

 

[Wo0zy]

Hi @Scythe42,

Updated to latest build from 1st post (sorry it took so long), launched MP in debug mode to capture logs and the problem was still there (GUI rendering @32fps). Accidentally shutdown the system on MP exit (instead of close) and on restart there were no logs on my desktop . However, after launching MP again, the problem has now gone (MP rendering GUI @ 50fps)???

Happy with the result but wondering why .

Will do some more testing tonight and will let you know if I find anything. In the meantime if there's anything you want me to check just let me know.

Sorry I haven't been able to provide any useful data. Only thing I can think to add is that prior to installing this build I was using the first "internal" 1.4 build which included your branch. Quite a lot has changed since then. Having said that, it doesn't explain why @HomeY still sees this problem. There must be something we're missing?

Cheers,

Wo0zy

Edit. And all of a sudden the issue returns..... . Debug logs attached. Hope they help.