search

ArcanePlugins / Treasury

🏦 A powerful multi-platform library for next-level plugin integrations.
https://hangar.papermc.io/ArcanePlugins/Treasury
Other
56 stars 13 forks source link

Nag about "not properly shutting down tasks" #234

Closed MrIvanPlays closed 1 year ago

MrIvanPlays commented 1 year ago

[16:37:00] [Server thread/ERROR]: Nag author: 'lokka30' of 'Treasury' about the following: This plugin is not properly shutting down its async tasks when it is being shut down. This task may throw errors during the final shutdown logs and might not complete before process dies

Happens on Paper. Treasury v2 only

Did some investigation, couldn't find any code that doesn't shut down tasks.

Bukkit sucks, prove me wrong

MrIvanPlays commented 1 year ago

This is some severe bullshit. Whilst yes I found the placeholder api hook tasks were not shutting down and i fixed it (commit not pushed yet), the nagging continues. Another weird problem is that I made it stdout all pending tasks. (Un)fortunately nothing was logged.

lokka30 commented 1 year ago

I guess I'll be nagged. 😆

Shall I bring this to the attention of Paper maintainers in case this is an error in their lingering async task detection? I doubt they have the time to do anything about it unless we can prove it is a Paper problem. ( ... vice-versa with PlaceholderAPI.)

MrIvanPlays commented 1 year ago

Ye we need paper's assistance here. Can you make some1 from their team have a look at our code? If they can't find anything wrong with it then it is not our problem.

lokka30 commented 1 year ago

Paper maintainers don't have much spare time, and neither me or them want to waste it, so I will need to do some testing first to try isolate where the issue is coming from. If I find anything then I'll report that to Paper, otherwise it'll likely be a "yes, Paper issue", "no, Treasury issue", "i don't know" answer.

MrIvanPlays commented 1 year ago

I've isolated the problem to be inside the PlaceholderAPI hook. Removing PlaceholderAPI from starting fixes the issue. I'll investigate further and probably fix the issue.

MrIvanPlays commented 1 year ago

@A248 you're the master of concurrency and stuff - if you can help here, do it. thx

A248 commented 1 year ago

Alright, sure. May I take advantage of the commit you mentioned here?

Whilst yes I found the placeholder api hook tasks were not shutting down and i fixed it (commit not pushed yet), the nagging continues

Maybe create a new branch to work on this bug. I'll PR to it.

MrIvanPlays commented 1 year ago

It is already pushed. Never mentioned it here

MrIvanPlays commented 1 year ago

@A248 any updates so far?

A248 commented 1 year ago

No, I haven't started working on this yet. I likely won't have time for it until spring break (March 5). If you'd like to work on it before then, feel free.

MrIvanPlays commented 1 year ago

oddly enough I cannot reproduce this for 1.19.3 . I'm just gonna close this issue.