mirror of
https://github.com/quasar/Quasar.git
synced 2026-04-25 23:35:58 +03:00
[GH-ISSUE #231] Form listviewitem null reference #101
Labels
No labels
bug
bug
cant-reproduce
discussion
duplicate
easy
enhancement
help wanted
improvement
invalid
need more info
pull-request
question
wont-add
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/Quasar#101
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @ghost on GitHub (May 27, 2015).
Original GitHub issue: https://github.com/quasar/Quasar/issues/231
Originally assigned to: @MaxXor on GitHub.
I have no clue why this is happening, but i installed 50 clients on a VM, and tried updating them all at once. The first time my server crashed and stopped responding. I tried debugging it but it wasn't throwing any exceptions when debugging. The only lead I have is from trying to disconnect an item that is still in the listview and it is claiming "object reference not set to an instance of an object."
any ideas?
@MaxXor commented on GitHub (May 27, 2015):
Hm, tested it on Windows 7 and working fine. What settings did you use for the 50 clients? And what for the update?
@ghost commented on GitHub (May 27, 2015):
Normal settings, no installation, different mutex for each, keylogger disabled.
Selected all 50, update from file, server stopped responding after it got down to ~25
The second time, it went down to ~15 clients. This time, the clients were still in the listview and the server did not crash, however when trying to execute a command on the client I would get a null reference exception dialog claiming the object had already been disposed
Same settings for the update
@ghost commented on GitHub (May 27, 2015):
Just got an exception in debugger
ListViewItem is null
@MaxXor commented on GitHub (May 27, 2015):
What's
null, theListViewitem, theTagorclient??@ghost commented on GitHub (May 27, 2015):
@MaxXor, ListViewItem
@ghost commented on GitHub (May 27, 2015):
Would placing a lock be appropriate in a case like this? I think it's caused by clients disconnecting asynchronously while the delegate method has a reference to a listviewitem that is no longer created
@MaxXor commented on GitHub (May 27, 2015):
Can you check if it's working with a
lock? Could be the reason.@ghost commented on GitHub (May 27, 2015):
Sure, will try now
@ghost commented on GitHub (May 27, 2015):
I'm sorry, I've no idea where to place the lock or what object to lock.
I'm assuming lstClients would be the object to lock!
@MaxXor commented on GitHub (May 27, 2015):
place the
lock (lstClients)right before the foreach-loop till the end of the loop.@ghost commented on GitHub (May 27, 2015):
Hmmm nope, still happening :(
Placed a lock on lstClients around the loop and still getting null reference
@MaxXor commented on GitHub (May 27, 2015):
I can't reproduce this still, do you use Upload Update or Update via Download from website?
@ghost commented on GitHub (May 27, 2015):
Update from file, I'm looking there now in the code and wondering if the threading has something to do with it.
@MaxXor commented on GitHub (May 27, 2015):
The thread is just there so the upload process doesn't freeze the form, this shouldn't be the problem..
I can't reproduce this with 40 test clients. :( It's actually no real scenario with all the clients on one machine.
@ghost commented on GitHub (May 27, 2015):
Hmmmm, well it doesn't happen every time, it's happened 3 times out of 6 tests. It is really weird that the listviewitem is nulled... I will keep trying to see what might be happening
@ghost commented on GitHub (May 27, 2015):
One thing that I've noticed, is after highlighting the all of the ListViewItems and updating, some of the listview items de-select (aren't highlighted) when the clients start to update and disconnect
@MaxXor commented on GitHub (May 27, 2015):
@d3agle that's no problem as the selected clients will be copied into a list. :)
https://github.com/MaxXor/xRAT/blob/master/Server/Forms/FrmMain.cs#L358
@ghost commented on GitHub (May 27, 2015):
I noticed that :) I'm stepping through to see what might be happening lower into it the code
@yankejustin commented on GitHub (May 27, 2015):
@MaxXor @d3agle Placing a lock on
lstClientsis useless. The lock is violated wheneverlstClientschanges. Consider creating aconstorreadonlyobject for locking. Ensure the object used to lock will not change.@yankejustin commented on GitHub (May 27, 2015):
I am actually going to re-do this quite a bit (similar to the RemoteShell changes I just made). Though invoking on the form works, we really need to separate the logic that should be kept in the form (GUI-modifying) with the things that really should be kept separated entirely.
@ghost commented on GitHub (May 27, 2015):
I will try that. Just wanted to point this out, not sure if its placebo or perhaps the cause, but I get the null reference exception when I use my middle mouse wheel to scroll through the list while the clients are disconnecting/updating. Scrolling through the list (with mouse wheel) while the clients are selected I notice that one of them/some of them de-select. When I update them all, and quickly un-select them all, I don't get any reference thrown. If I leave them all selected and don't scroll through the list, I get no exception.
@yankejustin commented on GitHub (May 27, 2015):
To also answer your question above, yes, this is a threading issue. :)
@MaxXor commented on GitHub (May 28, 2015):
Would you mind checking the issue after the latest commit?
github.com/MaxXor/xRAT@8a6709ba9f@ghost commented on GitHub (May 28, 2015):
It it still happening. I do believe this is a threading issue, due to the inconsistency of being able to reproduce this bug. It happens quite a bit when I highlight 50 clients -> update from file -> de-select them all, and then select some of them as they are disconnecting.
@ghost commented on GitHub (May 28, 2015):
Another strange thing I've noticed that is probably related to this is when I disconnected 100 clients at once, this was the result. Notice the text is drawn differently in one listviewitem
@MaxXor commented on GitHub (May 28, 2015):
Working on a fix for this.
@ghost commented on GitHub (May 29, 2015):
Unfortunately, the problem persists :( The behavior is worse after the update.
@MaxXor commented on GitHub (May 29, 2015):
@d3agle Why worse?
@ghost commented on GitHub (May 29, 2015):
This is what it is like closing the server: http://i.gyazo.com/63622aefcab03cfeb09e9b43e451097d.mp4
http://gyazo.com/abfa94f451e178aa046d5c42ee1a31e6.mp4
"Cross-thread operation not valid: Control 'lstClients' accessed from a thread other than the thread it was created on."
The problem I was referring to is the behavior when updating/disconnecting clients. sometimes the listviewitem will stay in the listview even though the client is disconnected
@MaxXor commented on GitHub (May 29, 2015):
The closing server behaviour was the same before, lol. Well, yea I'm still working on a final fix for this disconnection bug.
@ghost commented on GitHub (May 29, 2015):
Oh ok, I hadn't really tested closing the server prior to this update. I'm stumped on why this is happening, but I hope you can fix it! It's so annoying lol
@MaxXor commented on GitHub (May 29, 2015):
Yes, sure. The server just tries to make a clean disconnect from every client. x)
@yankejustin commented on GitHub (May 29, 2015):
@d3agle Please try it out now. As I have said before, locking on
_clientsis actually useless in a spot that messes with the object.Please see the example below. :)
Example:
1. Acquire a lock on_clients. 2. Disconnecting user, so remove entry 0 from_clients. 3. Another thread tries to enter. The previous entering thread acquired a lock on_clients, but_clientshas been changed, so it may enter (when it shouldn't be able to).With that example, that is why I lock on objects that I know don't change, and why it is important to prevent changes to the object used to lock.
@ghost commented on GitHub (May 29, 2015):
I tried your branch and I think we are closer to the culprit.
I installed 100 clients, and tried reconnecting them all at once. 5 of them stayed in the listview and I was given an exception here: System.NullReferenceException: Object reference not set to an instance of an object at 'xServer.Core.Misc.ListViewColumnSorter.Compare(Object x, Object y) at line 51.
@yankejustin commented on GitHub (May 29, 2015):
I forgot to implement a lock for
GetSelectedClients. Couldn't sort correctly because of multiple access tolstClients. x)Try again. :)
If needed, add to my pull request.
@ghost commented on GitHub (May 29, 2015):
Got deadlock somewhere :O
@yankejustin, this happened after locking GetSelectedClients method.
Form UI freezes
@ghost commented on GitHub (May 29, 2015):
Strangely enough, this happens when 6 clients are selected. When 5 clients are selected, it will leave one unhighlighted. Anything <= 4 will function fine :|
@yankejustin commented on GitHub (May 29, 2015):
@d3agle Please try it again and see if this is better.
@ghost commented on GitHub (May 29, 2015):
@yankejustin, My previous comments were towards the most recent commit,
github.com/yankejustin/xRAT@cdfe93f69aPrior to locking in the GetSelectedClients method it worked fine (didn't get deadlock after sending commands to less than 4 clients, 5 had weird behavior, 6 or more would cause a deadlock)
@yankejustin commented on GitHub (May 29, 2015):
@d3agle Right, I forgot to clarify that I just changed it 16 minutes ago. Try withthe newest change and see if it still deadlocks.
@yankejustin commented on GitHub (May 29, 2015):
If it still does not work then I think I have a strong idea what will. :)
@ghost commented on GitHub (May 29, 2015):
I don't see any changes from the last commit :X
@yankejustin commented on GitHub (May 29, 2015):
@d3agle Sorry, forgot to press sync! x)
@ghost commented on GitHub (May 29, 2015):
:D no problem, testing now
@yankejustin commented on GitHub (May 29, 2015):
Mind if I ask your exact methods of testing?
@ghost commented on GitHub (May 29, 2015):
Well, since I was too lazy to generate random mutex in the code for the builder, I spend about 5 minutes creating 100 clients name 1.exe - 100.exe. Put them in a folder -> Copied to VM -> executed them all. Then I test them without debugger first, to see what runtime errors might occur trying various different things (Mainly habitual things). Then if any errors occur, I test the same situations with debugger to see what line of code is breaking.
@ghost commented on GitHub (May 29, 2015):
Mainly looking for behavior consistency/inconsistency
@yankejustin commented on GitHub (May 29, 2015):
Ew... I see a cross-thread exception in
GetSelectedClients()that is in theMaxXor/masterbranch. I am going to fix that very quick.@ghost commented on GitHub (May 29, 2015):
Ok, the deadlock is still occuring, however it doesn't happen as often. It appears to happen when about ~60 clients have disconnected
And quite a large memory leak it seems :O
@yankejustin commented on GitHub (May 29, 2015):
What are you doing before it locks?
@ghost commented on GitHub (May 29, 2015):
I'm selecting all the clients, and sending reconnect/disconnect commands. It's locking at random times
@ghost commented on GitHub (May 29, 2015):
I don't touch anything after I send the commands
@MaxXor commented on GitHub (May 29, 2015):
I have a fix ready, please don't work on this now. :)
@ghost commented on GitHub (May 29, 2015):
Sounds good 👍
@yankejustin commented on GitHub (May 29, 2015):
Oh, okay...
@yankejustin commented on GitHub (May 29, 2015):
That is funny because I just fixed the eventhandlers and I believe the rest, @d3agle xD
@MaxXor commented on GitHub (May 29, 2015):
I made a lot more changes.
@yankejustin commented on GitHub (May 29, 2015):
Looks delicious. Nice work. :)
@MaxXor commented on GitHub (May 29, 2015):
Thanks, this should also fix other cross-threading problems which may occur too.
@ghost commented on GitHub (May 29, 2015):
I cannot break it. Good stuff. (Updated clients with YSG.exe)
http://i.gyazo.com/e1e73e57e3a30b78d5f82d5287832d1b.mp4
@yankejustin commented on GitHub (May 29, 2015):
Gorgeous. :)
I do eventually want to use a bit of
SuspendLayout()andResumeLayout()so it doesn't flicker so much like that.@ghost commented on GitHub (May 29, 2015):
@yankejustin, take a look at this as well http://www.virtualdub.org/blog/pivot/entry.php?id=273
Perhaps this could be useful :D
@yankejustin commented on GitHub (May 29, 2015):
LOL... "ListViewWithLessSuck"
That made my day. :)
Eventually I do want to give some of the controls some love where it is needed, but that could be a bit of a pain; toying with controls takes so much time.
@ghost commented on GitHub (May 29, 2015):
That's true :( but it would definitely be an appealing touch. I haven't looked into theming extensively, but it might be a fun little side-project. It would be really cool if we had features to have different themes/color schemes. But the flickering is pretty annoying, just looks unclean lol.
@yankejustin commented on GitHub (May 29, 2015):
Yes, I agree. It is something I feel is important for the next version. I want to make it look much better, but that will certainly take some effort.
@MaxXor commented on GitHub (May 30, 2015):
Just add your new Listview code to this file: https://github.com/MaxXor/xRAT/blob/master/Server/Controls/ListViewEx.cs
And see if the flickering is gone. :D
@yankejustin commented on GitHub (May 30, 2015):
👍 💩 👍
@ghost commented on GitHub (May 31, 2015):
Is this what you guys mean by flicker? I'm still noticing some weird behavior
http://i.gyazo.com/6fed8b019d8d2f20c9e639206a67aff6.mp4
@MaxXor commented on GitHub (May 31, 2015):
This is the GUI (listview in this case) which updates when each client gets removed from the list, not the real flickering like it was before.